Community
Participate
Working Groups
There are some cases where we know that an submit must fail. 1) anonymous user can not submit bugs 2) when ../show_bug.cgi?ctype=xml&id= did not have the exporter attribute in <bugzilla> then all emails did not have @domain We should show a message with an hyperlink for the action. patch is coming soon!
Created attachment 195009 [details] patch V1 committed patch
Created attachment 195010 [details] mylyn/context/zip
I'm -1 on the API change to AbstractTaskEditorPage. The problem is that there a multiple things that may need to disable the submit button and making that available through API will lead to more inconsistencies. Frank, can you please revert the API change to AbstractTaskEditorPage? I like the idea of the patch but it should be sufficient to show an error on submit as we do when the description is empty for instance. I would also suggest to rename testCanSubmit() to checkCanSubmit() or something like that. It's a common convention to prefix JUnit test methods with "test" and it can be confusing if that prefix is used in other contexts.
Created attachment 195019 [details] patch V2 committed patch Now we show informational when we create the editor and errors when doSubmit is executed.
Created attachment 195020 [details] mylyn/context/zip
(In reply to comment #3) > I'm -1 on the API change to AbstractTaskEditorPage. The problem is that there a > multiple things that may need to disable the submit button and making that > available through API will lead to more inconsistencies. Frank, can you please > revert the API change to AbstractTaskEditorPage? API change reverted! > > I like the idea of the patch but it should be sufficient to show an error on > submit as we do when the description is empty for instance. > > I would also suggest to rename testCanSubmit() to checkCanSubmit() or something > like that. It's a common convention to prefix JUnit test methods with "test" and > it can be confusing if that prefix is used in other contexts. method renamed!
Frank, I seem to be seeing this on repositories where I wouldn't expect this type of error to occur as there is no anonymous login. I think that this could be due to my network being down during a sync of the tasks.
(In reply to comment #7) > Frank, I seem to be seeing this on repositories where I wouldn't expect this > type of error to occur as there is no anonymous login. I think that this could > be due to my network being down during a sync of the tasks. I bet that you get "submit disabled, please refresh". If so the reason is that we have a new attribute in the taskdata (exporter). If exporter is not in taskdata we have no @domin in the email adress so we can not submit. You need to do a refresh and then you can submit the task.
We need to fix this so the message does not show for existing tasks. We also need to make the error message more meaningful and include details about the cause. Frank, are your sure that all Bugzilla versions have the supporter attribute? I am bit afraid that we may break submission for a lot of repositories with this change.
Yes, i currently have to refresh every single task that I have before submit :(
(In reply to comment #9) > We need to fix this so the message does not show for existing tasks. We also > need to make the error message more meaningful and include details about the > cause. > > Frank, are your sure that all Bugzilla versions have the supporter attribute? I > am bit afraid that we may break submission for a lot of repositories with this > change. Yes I have checked thsi with all ower test repositories. Here the header of the oldest version <bugzilla version="2.22.7" urlbase="" maintainer="THE MAINTAINER HAS NOT YET BEEN SET" exporter="test@mylyn.eclipse.org" > (In reply to comment #10) > Yes, i currently have to refresh every single task that I have before submit :( OK, we need to find a way to avoid this.
Created attachment 195554 [details] reverted change
Frank, I need to do a weekly build and have reverted the change for now until we have come up with a solution for the migration problem.
Created attachment 195651 [details] patch V3 committed patch with an solution for the migration problem.
Created attachment 195652 [details] mylyn/context/zip
Frank, it looks like the latest iteration is doing an extra synchronization prior to submission? I'm worried that this could cause merge conflicts or missed incomings. Unless we find a solution that does not require extra network activity, e.g. through using migrateTaskData() I don't think we should go ahead with this change.
(In reply to comment #16) > Frank, it looks like the latest iteration is doing an extra synchronization > prior to submission? I'm worried that this could cause merge conflicts or > missed incomings. Unless we find a solution that does not require extra network > activity, e.g. through using migrateTaskData() I don't think we should go ahead > with this change. No the extra synchronization is only when the URLBASE Attribute does not exits. The extra synchronization is triggered during open of the editor. If we want to use the migrate which value should we use for the EXPORTER? The user of the configuration could be used. But in some cases we can have email without the @domain and user of the configuration set. This is an error but i think it is not so common. Steffen, should I create a fix for this?
I'm now seeing flickering everytime I open an editor. Can we please revert this change and work with patches until we have arrived at a solution that does not cause regressions? Not sure what a sensible value for EXPORTER is. Maybe the reporter?
(In reply to comment #18) > I'm now seeing flickering everytime I open an editor. Can we please revert this > change and work with patches until we have arrived at a solution that does not > cause regressions? OK patch V3 is now reverted! > > Not sure what a sensible value for EXPORTER is. Maybe the reporter? No the EXPORTER is the name of the logged in user how requested the xml from bugzilla.
(In reply to comment #19) > > Not sure what a sensible value for EXPORTER is. Maybe the reporter? > > No the EXPORTER is the name of the logged in user how requested the xml from > bugzilla. Interesting. Could we use the user from the task repository object in that case?
Created attachment 195681 [details] fix compile error
Created attachment 195682 [details] mylyn/context/zip
Created attachment 195684 [details] mylyn/context/zip
Created attachment 198845 [details] patch V4 Here the patch for backport to 3.6.1 This patch is now in HEAD
Created attachment 198846 [details] mylyn/context/zip
Thanks. Since there is a risk of regressions I would prefer if we apply this change to master only for now. I am not sure that it's worth the risk anyways but we can still consider backporting later if the change does not cause any unexpected problems.
