Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 172021

Summary: [api] AbstractRepositoryTaskEditor should optionally support delete/remove for comments and attachments
Product: z_Archived Reporter: Lubos Pochman <pochmans>
Component: MylynAssignee: Lubos Pochman <pochmans>
Status: RESOLVED FIXED QA Contact:
Severity: enhancement    
Priority: P5 CC: pochmans, robert.elves, shawn.minto
Version: devKeywords: helpwanted
Target Milestone: ---   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Patch for deleting attachments and comments
none
Image of delete attachment, comment controls
none
Delete comment icon
none
New patch for deleting attachments and comments none

Description Lubos Pochman CLA 2007-01-29 10:20:12 EST
Some connectors support deleting/removing comments and attachments.
AbstractRepositoryTaskEditor should optionally support delete/remove for comments and attachments maybe with
Delete button next to each attachment or comment.
Currently this has to be solved by adding custom attribute for deleting attachment or comment.
Comment 1 Mik Kersten CLA 2007-01-29 14:47:59 EST
Lubos: are you interested in contributing this?  We don't have a driver because we don't internally support a repository that has this feature.
Comment 2 Eugene Kuleshov CLA 2007-01-29 15:09:24 EST
Mik, jira does allow deleting attachments. Though I am not sure if it is exposed in the API...
Comment 3 Lubos Pochman CLA 2007-01-29 15:20:32 EST
I would be glad to contribute it. What timeframe? 2.0M1 or 2.0M2 or ...?
Comment 4 Mik Kersten CLA 2007-02-01 17:02:37 EST
Lubos: since you're volunteering to take this one on, the time frame is up to you.  We try to integrate contributors' patches as quickly as we can.

What you should do to start is overview how you want to implement it and try to keep the API changes minimal.  We'll comment, and then you can go ahead and create a patch.
Comment 5 Mik Kersten CLA 2007-02-12 13:58:28 EST
Lubos: are you considering doing this?  If not let me know and I'll reassign it to mylar-inbox.
Comment 6 Lubos Pochman CLA 2007-02-12 14:22:25 EST
I will do that. I am waiting for M5 Eclipse/Mylar to start going.
Comment 7 Mik Kersten CLA 2007-02-12 14:57:14 EST
Great.  The last dev build we did does support M5, so you can move to that.  But we'll be releasing an updated M5 dev build today.
Comment 8 Mik Kersten CLA 2007-03-18 22:29:49 EDT
Lubos: if you are still planning on doing this just let us know.  Lowering priority for now.
Comment 9 Lubos Pochman CLA 2007-03-20 18:33:45 EDT
I will start working on it. I will wait for M6 and stable Mylar M6 based build before starting work on it. Should be end of this week, right?
Comment 10 Mik Kersten CLA 2007-03-21 13:07:58 EDT
Great.  Yes, M6 is this week and Mylar dev builds already support it.  However, you would be best off to wait until we release Mylar 2.0M2 next Friday since there might be some API changes coming up.
Comment 11 Lubos Pochman CLA 2007-04-10 15:39:59 EDT
Created attachment 63421 [details]
Patch for deleting attachments and comments
Comment 12 Lubos Pochman CLA 2007-04-10 15:40:52 EDT
Created attachment 63422 [details]
Image of delete attachment, comment controls
Comment 13 Lubos Pochman CLA 2007-04-10 15:51:16 EDT
Created attachment 63424 [details]
Delete comment icon
Comment 14 Lubos Pochman CLA 2007-04-10 15:51:58 EDT
I have added patch (need delete comment image if accepted) for deleting attachments and comments. UI controls can be seen in attached image.
Four protected methods has been introduced. By default they do not support delete comment and/or attachment (no changes in the existing code are needed).
If connector supports delete comments and/or attachment it needs to override (and implement) those methods:

protected boolean supportsAttachmentDelete();
protected void deleteAttachment(RepositoryAttachment attachment);
protected boolean supportsCommentDelete();
protected void deleteComment(TaskComment comment);

Attached Delete comment button icon should be placed in org.eclipse.mylar.tasks.ui\icons\elcl16\comment_delete.png.

Could one of the committers please review the patch. Thanks.
Comment 15 Mik Kersten CLA 2007-04-10 20:32:07 EDT
Rob: please review.
Comment 16 Lubos Pochman CLA 2007-04-11 09:40:53 EDT
Rob, I think there should be a call in "select" callbacks that delete an attachment or comment to force refresh the task after the delete. What is the call to refresh the task?
Comment 17 Robert Elves CLA 2007-04-11 17:18:39 EDT
New task data will be retrieved by calling:
TasksUiPlugin.getSynchronizationManager().synchronize(connector, modifiedTask, true,....

...Adding a job listener to call AbstractRepositoryTaskEditor.refreshEditor(), which will refresh the editor contents with the recently synched task data (an example can be found in AbstractRepositoryTaskEditor.submitToRepository()).

Let me know if you are considering submitting an updated patch.
Comment 18 Lubos Pochman CLA 2007-04-13 16:45:18 EDT
Created attachment 63795 [details]
New patch for deleting attachments and comments
Comment 19 Lubos Pochman CLA 2007-04-13 16:48:54 EDT
Rob, attached is new version of patch that handles the task synchronization after attachment or comment delete.
Attached Delete comment button icon should be placed in
org.eclipse.mylar.tasks.ui\icons\elcl16\comment_delete.png. (Patch doesn't handle binary files).

If you are OK with implementation, would you please commit the patch and let me know, so I can check it out and validate.

Thanks, Lubos
Comment 20 Robert Elves CLA 2007-04-13 20:58:08 EDT
Patch applied (missed the dev build, but we can cut one for you next week). Used standard 'remove' icon for the remove comment link.  Moved button next to reply link.
Comment 21 Robert Elves CLA 2007-04-17 17:42:31 EDT
Lubos: A new dev build is available for you to test against: http://download.eclipse.org/technology/mylar/update-site/dev/e3.3/
Comment 22 Lubos Pochman CLA 2007-04-17 19:21:28 EDT
Rob, you are the man! Tested and validated. Looks great. I am marking the bug as fixed.