Bug 349771 - support marking comments private
Summary: support marking comments private
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 3.7   Edit
Assignee: Frank Becker CLA
QA Contact: Frank Becker CLA
URL:
Whiteboard:
Keywords: plan
: 224119 (view as bug list)
Depends on: 349620
Blocks: 371158
  Show dependency tree
 
Reported: 2011-06-19 12:22 EDT by Frank Becker CLA
Modified: 2012-02-16 16:43 EST (History)
2 users (show)

See Also:


Attachments
patch V1 (4.81 KB, patch)
2011-06-28 13:39 EDT, Frank Becker CLA
no flags Details | Diff
mylyn/context/zip (15.31 KB, application/octet-stream)
2011-06-28 13:39 EDT, Frank Becker CLA
no flags Details
patch V2 (2.09 KB, patch)
2011-06-28 15:37 EDT, Frank Becker CLA
no flags Details | Diff
mylyn/context/zip (2.77 KB, application/octet-stream)
2011-06-28 15:37 EDT, Frank Becker CLA
no flags Details
patch V3 (1.75 KB, patch)
2011-06-30 23:43 EDT, Frank Becker CLA
no flags Details | Diff
mylyn/context/zip (4.63 KB, application/octet-stream)
2011-06-30 23:43 EDT, Frank Becker CLA
no flags Details
Bugzilla UI for private comments (4.86 KB, image/png)
2011-07-28 09:59 EDT, Steffen Pingel CLA
no flags Details
Bugzilla Connector UI (13.96 KB, image/png)
2011-07-28 10:14 EDT, Steffen Pingel CLA
no flags Details
JIRA example (7.57 KB, image/png)
2011-09-22 06:58 EDT, Steffen Pingel CLA
no flags Details
mylyn/context/zip (10.19 KB, application/octet-stream)
2011-10-03 15:43 EDT, Frank Becker CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Frank Becker CLA 2011-06-19 12:22:19 EDT
Actual we can only change existing comments and not the new comment.
Comment 1 Sam Davis CLA 2011-06-27 18:21:03 EDT
I'm getting the following NPE when trying to create a new task:

java.lang.NullPointerException
	at org.eclipse.mylyn.internal.bugzilla.ui.editor.BugzillaTaskEditorNewCommentPart.initialize(BugzillaTaskEditorNewCommentPart.java:67)
	at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage.initializePart(AbstractTaskEditorPage.java:1292)
	at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage.access$7(AbstractTaskEditorPage.java:1289)
	at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage$14.run(AbstractTaskEditorPage.java:850)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
	at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage.createParts(AbstractTaskEditorPage.java:841)
	at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage.createParts(AbstractTaskEditorPage.java:826)
	at org.eclipse.mylyn.internal.bugzilla.ui.editor.BugzillaTaskEditorPage.createParts(BugzillaTaskEditorPage.java:339)
	at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage.createFormContentInternal(AbstractTaskEditorPage.java:712)
	at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage.createFormContent(AbstractTaskEditorPage.java:657)
	at org.eclipse.ui.forms.editor.FormPage$1.run(FormPage.java:152)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70)
	at org.eclipse.ui.forms.editor.FormPage.createPartControl(FormPage.java:150)
	at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage.createPartControl(AbstractTaskEditorPage.java:610)
	at org.eclipse.ui.forms.editor.FormEditor.pageChange(FormEditor.java:471)
	at org.eclipse.ui.part.MultiPageEditorPart.setActivePage(MultiPageEditorPart.java:1067)
	at org.eclipse.ui.forms.editor.FormEditor.setActivePage(FormEditor.java:603)
	at org.eclipse.ui.forms.editor.SharedHeaderFormEditor.setActivePage(SharedHeaderFormEditor.java:110)
	at org.eclipse.mylyn.tasks.ui.editors.TaskEditor.addPages(TaskEditor.java:407)
	at org.eclipse.ui.forms.editor.FormEditor.createPages(FormEditor.java:138)
	at org.eclipse.ui.forms.editor.SharedHeaderFormEditor.createPages(SharedHeaderFormEditor.java:98)
	at org.eclipse.ui.part.MultiPageEditorPart.createPartControl(MultiPageEditorPart.java:348)
	at org.eclipse.ui.internal.EditorReference.createPartHelper(EditorReference.java:670)
	at org.eclipse.ui.internal.EditorReference.createPart(EditorReference.java:465)
	at org.eclipse.ui.internal.WorkbenchPartReference.getPart(WorkbenchPartReference.java:595)
	at org.eclipse.ui.internal.EditorReference.getEditor(EditorReference.java:289)
	at org.eclipse.ui.internal.WorkbenchPage.busyOpenEditorBatched(WorkbenchPage.java:2863)
	at org.eclipse.ui.internal.WorkbenchPage.busyOpenEditor(WorkbenchPage.java:2768)
	at org.eclipse.ui.internal.WorkbenchPage.access$11(WorkbenchPage.java:2760)
	at org.eclipse.ui.internal.WorkbenchPage$10.run(WorkbenchPage.java:2711)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70)
	at org.eclipse.ui.internal.WorkbenchPage.openEditor(WorkbenchPage.java:2707)
	at org.eclipse.ui.internal.WorkbenchPage.openEditor(WorkbenchPage.java:2691)
	at org.eclipse.ui.internal.WorkbenchPage.openEditor(WorkbenchPage.java:2674)
	at org.eclipse.mylyn.tasks.ui.TasksUiUtil.openEditor(TasksUiUtil.java:176)
	at org.eclipse.mylyn.internal.tasks.ui.util.TasksUiInternal.createAndOpenNewTask(TasksUiInternal.java:863)
	at org.eclipse.mylyn.tasks.ui.wizards.NewTaskWizard.performFinish(NewTaskWizard.java:130)
	at org.eclipse.mylyn.tasks.ui.TasksUiUtil.openNewTaskEditor(TasksUiUtil.java:228)
	at org.eclipse.mylyn.tasks.ui.TasksUiUtil.openNewTaskEditor(TasksUiUtil.java:254)
	at org.eclipse.mylyn.internal.tasks.ui.actions.NewTaskAction$RepositorySelectionAction.run(NewTaskAction.java:248)
	at org.eclipse.jface.action.Action.runWithEvent(Action.java:498)
	at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:584)
	at org.eclipse.jface.action.ActionContributionItem.access$2(ActionContributionItem.java:501)
	at org.eclipse.jface.action.ActionContributionItem$5.handleEvent(ActionContributionItem.java:411)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1053)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4066)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3657)
	at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2640)
	at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2604)
	at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2438)
	at org.eclipse.ui.internal.Workbench$7.run(Workbench.java:671)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:664)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:115)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:369)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.lang.reflect.Method.invoke(Unknown Source)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:619)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:574)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1407)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1383)
Comment 2 Sam Davis CLA 2011-06-27 18:22:03 EDT
The message is "Error creating task editor part: "org.eclipse.mylyn.tasks.ui.editors.parts.newComment"" Should it even be creating this part when creating a new task?
Comment 3 Frank Becker CLA 2011-06-28 13:39:00 EDT
Created attachment 198759 [details]
patch V1

Here my committed changes for the reported problem.

Sorry!
Comment 4 Frank Becker CLA 2011-06-28 13:39:02 EDT
Created attachment 198760 [details]
mylyn/context/zip
Comment 5 Frank Becker CLA 2011-06-28 15:37:56 EDT
Created attachment 198771 [details]
patch V2

Sorry,

there was one file missing in patch V1
Comment 6 Frank Becker CLA 2011-06-28 15:37:59 EDT
Created attachment 198772 [details]
mylyn/context/zip
Comment 7 Steffen Pingel CLA 2011-06-30 19:08:52 EDT
Frank, I am afraid the changes to AbstractTaskEditorPage will break other connectors. Can you please revert them? We'll need to make the Bugzilla connector work so that it works without these changes.
Comment 8 Frank Becker CLA 2011-06-30 23:43:47 EDT
Created attachment 198942 [details]
patch V3

(In reply to comment #7)
> Frank, I am afraid the changes to AbstractTaskEditorPage will break other
> connectors. Can you please revert them? We'll need to make the Bugzilla
> connector work so that it works without these changes.

Changes are now reverted.

I think we do not need to chnage Bugzilla Conector because the changes where only to not create the ID_PART_NEW_COMMENT Part when we have existing bugs. For new Bugs the TaskAttribute is null so we do not show this to the UI.

I thought that we can remove the ID_PART_NEW_COMMENT for new bugs.

Sorry.
Comment 9 Frank Becker CLA 2011-06-30 23:43:50 EDT
Created attachment 198943 [details]
mylyn/context/zip
Comment 10 Steffen Pingel CLA 2011-07-28 09:59:14 EDT
Created attachment 200529 [details]
Bugzilla UI for private comments
Comment 11 Steffen Pingel CLA 2011-07-28 10:14:36 EDT
Created attachment 200530 [details]
Bugzilla Connector UI
Comment 12 Steffen Pingel CLA 2011-07-28 10:15:05 EDT
I like the little lock icon! Some notes from a quick UI review:

* The setting in the Additional Settings should be renamed, e.g. to "Enable private comments:"
* The error handling is a bit problematic. There is no warning if a comment is submitted and could not be set as private (it appears as public).
* Task editor toolbar icons need to be 12x12 (or we need to change all icons to 16x16)
* Comment numbering does not match the web UI when accessing tasks outside of the insider group

For the future, we should consider moving the implementation into the framework since it is fairly common that repositories allow to restrict

I wonder if we should mark private comments even in collapsed mode by showing the lock.

I'll also put this on today's meeting agenda to gather some more feedback.
Comment 13 Frank Becker CLA 2011-07-31 12:26:25 EDT
I create bug https://bugzilla.mozilla.org/show_bug.cgi?id=675502 for the enhancements.

Hope that I can do the other thinks soon!
Comment 14 Frank Becker CLA 2011-07-31 16:19:35 EDT
Here the bug for the Webservice implementation https://bugzilla.mozilla.org/show_bug.cgi?id=675517
Comment 15 Steffen Pingel CLA 2011-08-11 14:38:22 EDT
*** Bug 224119 has been marked as a duplicate of this bug. ***
Comment 16 Steffen Pingel CLA 2011-09-22 06:58:12 EDT
Created attachment 203832 [details]
JIRA example
Comment 17 Steffen Pingel CLA 2011-09-22 07:00:13 EDT
Some more suggestions from another UI review on the call:

* Replace the lock icon with the standard Eclipse lock (the one that's used in the password dialog)
* Move the actions for toggling visibility into the drop-down menu and decorate comments that are private. I like how this was done for JIRA. I would suggested doing something similar. Mik suggested that we should always show the lock icon for private comments.
Comment 18 Frank Becker CLA 2011-10-02 16:21:06 EDT
(In reply to comment #17)
> Some more suggestions from another UI review on the call:
> 
> * Replace the lock icon with the standard Eclipse lock (the one that's used in
> the password dialog)
What icon did you mean?
/src/plugins/org.eclipse.jdt.junit/icons/full/elcl16/lock.gif
/src/plugins/org.eclipse.ui/icons/full/progress/lockedstate.gif
/src/plugins/org.eclipse.team.ui/icons/full/wizban/lockkey.gif

> * Move the actions for toggling visibility into the drop-down menu and decorate
> comments that are private. I like how this was done for JIRA. I would suggested
> doing something similar. Mik suggested that we should always show the lock icon
> for private comments.
We can add something like "Visible to Members of the Insidergroup" or should we stay with the icons?
Comment 19 Frank Becker CLA 2011-10-03 15:43:48 EDT
I create http://review.mylyn.org/#change,71 Steffen please check if this is the right way!
Comment 20 Frank Becker CLA 2011-10-03 15:43:52 EDT
Created attachment 204470 [details]
mylyn/context/zip
Comment 21 Steffen Pingel CLA 2011-10-04 09:37:23 EDT
(In reply to comment #18)
> > * Replace the lock icon with the standard Eclipse lock (the one that's used in
> > the password dialog)
> What icon did you mean?
> /src/plugins/org.eclipse.jdt.junit/icons/full/elcl16/lock.gif
> /src/plugins/org.eclipse.ui/icons/full/progress/lockedstate.gif
> /src/plugins/org.eclipse.team.ui/icons/full/wizban/lockkey.gif

Good question. I'll ask Mik to clarify. I assume it's the last icon but we would need to get create a scaled version. We should consider revolving bug 280564 first before we invest any time into that.

> We can add something like "Visible to Members of the Insidergroup" or should we
> stay with the icons?

I think we want both, an icon and a message.
Comment 22 Frank Becker CLA 2011-10-04 12:16:57 EDT
(In reply to comment #21)
> ...
> > We can add something like "Visible to Members of the Insidergroup" or should we
> > stay with the icons?
> 
> I think we want both, an icon and a message.
OK , then I have to add the text to the review
Comment 23 Steffen Pingel CLA 2012-02-09 19:34:27 EST
I think the lockkey.gif icon is the right icon that we want here. Can you rebase the patch set against the latest master?
Comment 24 Steffen Pingel CLA 2012-02-16 16:42:33 EST
I'll mark this bug as resolved since the key pieces are in place. We can track further improvements on bug 349620.

Thanks for getting the support for marking comments private into Mylyn 3.7!