Bug 411038

Summary: Gerrit reviews editor fails with NPE when new comment is submitted
Product: z_Archived Reporter: David Green <greensopinion>
Component: MylynAssignee: Miles Parker <milesparker>
Status: RESOLVED FIXED QA Contact:
Severity: major    
Priority: P3 CC: milesparker
Version: unspecified   
Target Milestone: 2.0.1   
Hardware: Macintosh   
OS: Mac OS X   
Whiteboard:

Description David Green CLA 2013-06-18 11:42:54 EDT 
To reproduce:
# open a Gerrit review in the review editor
# fill in a new comment in the "New Comment" area
# press Alt+Shift+S to submit
# observe "Submit failed: unexpected error: null"

Using Mylyn Reviews Connector: Gerrit	2.0.0.v20130612-0100

the following exception shows in the error log:

java.lang.UnsupportedOperationException
	at org.eclipse.mylyn.internal.gerrit.core.GerritTaskDataHandler.postTaskData(GerritTaskDataHandler.java:214)
	at org.eclipse.mylyn.internal.tasks.core.sync.SubmitTaskJob.run(SubmitTaskJob.java:100)
	at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)
Comment 1 Miles Parker CLA 2013-06-18 14:39:48 EDT 
Wow, good one. This only fails when one uses the keyboard combination, not when clicking the submit button.

I believe usage wasn't actually anticipated, because until we have support for a more Mylyn like workflow (see bug 370645, bug 384770), you can't actually "Submit" the task in the usual way -- you have to use "Publish Comments" from within a patch set. (Note that there isn't actually a Submit button in the header.) So it seems that what we need to do is disable the keyboard action or something like that.

(I'm not sure if we should tackle this one for 2.0.1 or hold for 2.1.)
Comment 2 Steffen Pingel CLA 2013-06-18 17:26:07 EDT 
This looks like a bug in the task editor to me. It should do nothing if the submit button is not enabled.
Comment 3 Miles Parker CLA 2013-06-18 18:04:16 EDT 
Actually the framework was working just fine. We just needed to set needsSubmit as well as needsSubmit button. It seems to me that we've discussed this one before so rather than have it come up again, I fixed it.

https://git.eclipse.org/r/#/c/13894/
Comment 4 Miles Parker CLA 2013-06-19 14:47:35 EDT 
Merged: https://git.eclipse.org/r/#/c/13894/
Comment 5 Steffen Pingel CLA 2013-06-19 17:26:49 EDT 
Assignee?
Comment 6 Miles Parker CLA 2013-06-19 18:01:01 EDT 
.