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

Bug 247745

Summary:

make CommentGroupStrategy testable

Product:

z_Archived

Reporter:

Jingwen 'Owen' Ou <jingweno>

Component:

Mylyn

Assignee:

Jingwen 'Owen' Ou <jingweno>

Status:

RESOLVED FIXED

QA Contact:

Severity:

enhancement

Priority:

CC:

steffen.pingel

Version:

unspecified

Target Milestone:

3.1

Hardware:

All

OS:

All

Whiteboard:

Bug Depends on:

Bug Blocks:

238038

Attachments:

Description	Flags
a patch that make CommentGroupStrategy testable	none
mylyn/context/zip	none
patch	none
mylyn/context/zip	none

Description Jingwen 'Owen' Ou

2008-09-17 17:56:31 EDT

see bug 244359 comment 7

Comment 1 Jingwen 'Owen' Ou

2008-09-17 17:58:54 EDT

Created attachment 112836 [details]
a patch that make CommentGroupStrategy testable

Comment 2 Jingwen 'Owen' Ou

2008-09-17 17:58:56 EDT

Created attachment 112837 [details]
mylyn/context/zip

Comment 3 Steffen Pingel

2008-09-20 17:57:58 EDT

Thanks Owen. I have committed a change along the lines of your patch that makes CommentGroupStrategy work based on ITaskComment only so it does not require a reference to TaskDataModel at all.

Comment 4 Steffen Pingel

2008-09-20 17:58:30 EDT

Created attachment 113070 [details]
patch

Comment 5 Steffen Pingel

2008-09-20 17:58:33 EDT

Created attachment 113071 [details]
mylyn/context/zip

Comment 6 Jingwen 'Owen' Ou

2008-09-21 03:47:23 EDT

(In reply to comment #3)
> Thanks Owen. I have committed a change along the lines of your patch that makes
> CommentGroupStrategy work based on ITaskComment only so it does not require a
> reference to TaskDataModel at all.

Wow...thats much better! Greate work, Steffen.

A minor point: If CommentGroupStrategy.hasIncomingChanges(..) always return false, it would be possible that  "current" & "recent" subsections are both toggled open, if current user's last comment is not the latest incoming comment. For example,

-Recent
	-incoming 1 <- this is first incoming comment
	-incoming 2
-Current
	-incoming 3 <- this is current user's latest comment
	-incoming 4
	
That's why we might need to ensure all incoming comments are grouped in the "current" subsection before we search the user's last comment (CommentGroupStrategy.hasIncomingChanges(..) should return true if it should).

Comment 7 Steffen Pingel

2008-09-21 15:23:14 EDT

ExtensibleTaskEditorCommentPart.getCommentGroupStrategy() overrides hasIncomingChanges() with the proper implementation that queries the model. I am still not sure if we should group all incomings in the current section. In the case of viewing a new bug that has 200 comments for instance this becomes very slow and most of the time the user is only interested in the most recent comments, i.e. it would be sufficient to expand Recent and Current.