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

Bug 322867

Summary:

[regression] target milestone field disappeared for Eclipse.org bugs

Product:

z_Archived

Reporter:

Steffen Pingel <steffen.pingel>

Component:

Mylyn

Assignee:

Frank Becker <eclipse>

Status:

RESOLVED FIXED

QA Contact:

Severity:

major

Priority:

CC:

ankur_sharma, eclipse, robert.elves

Version:

unspecified

Target Milestone:

3.5

Hardware:

OS:

Linux

Whiteboard:

Bug Depends on:

325190

Bug Blocks:

320202

Attachments:

Description	Flags
patch	none
mylyn/context/zip	none
commited patch	none
mylyn/context/zip	none
commited patch V3	none
mylyn/context/zip	none

Description Steffen Pingel

2010-08-17 01:35:53 EDT

After bootstrapping on the latest head I do not see the target milestone field on Eclipse.org bugs any longer. I had to toggle the option to suppress the field in the repository preferences to make the field appear.  There are two problems with that:

* Users will perceive this as a regression since the target milestone field simply disappears on synchronization of a task.
* The state displayed in the repository properties dialog does not actually reflect the behavior of the task editor (i.e. the target milestone was not checked, still the field was not displayed in the editor)

I also see a Classification field now that was not there before. 

I would strongly suggest that we maintain the Mylyn 3.4 behavior for backwards compatibility, i.e. suppress Classification and show Target Milestone by default.

Comment 1 Frank Becker

2010-08-17 16:27:23 EDT

Steffen,

sorry for this. 

You can fix this by open the Eclipse.org Tatsk Repository Properties.

I think we must define an other parameter that tell use if we use the new parameter. If this parameter did not exists we fall back th the old way.

Do you think that is OK?

Comment 2 Frank Becker

2010-08-19 16:47:17 EDT

Created attachment 177049 [details]
patch

Steffen,

can you please verify that this fix the problem?

Thanks.

Comment 3 Frank Becker

2010-08-19 16:47:20 EDT

Created attachment 177050 [details]
mylyn/context/zip

Comment 4 Steffen Pingel

2010-08-19 23:36:28 EDT

The idea of this patch looks right to me. I would prefer though if property accessors were encapsulated in a utility class to avoid code duplication between BugzillaRepositorySettingsPage and BugzillaClient, e.g. 

BugzillaUtil {

	public static boolean getUseTargetMilestone(Repository) {
		String useParam = taskRepository.getProperty(IBugzillaConstants.BUGZILLA_PARAM_USETARGETMILESTONE);
		return useParam == null || Boolean.parseBoolean(useParam);
	}

}

Comment 5 Frank Becker

2010-08-21 15:38:23 EDT

Created attachment 177162 [details]
commited patch

Comment 6 Frank Becker

2010-08-21 15:38:25 EDT

Created attachment 177163 [details]
mylyn/context/zip

Comment 7 Frank Becker

2010-08-21 15:39:49 EDT

patch is in HEAD

Comment 8 Steffen Pingel

2010-08-21 16:12:03 EDT

Thanks Frank. There is still some duplication in regard to handling properties and I wonder if the template handling might throw NPEs. Let's do another review once Rob gets back from vacation.

Comment 9 Steffen Pingel

2010-09-01 14:56:12 EDT

As far as I can tell the logic that determines whether an attribute has been configured or if the default is to be used is duplicated in several places. Frank, I firmly believe that we should extract that into a utility methods so there is a single place where the policy for the default is defined (comment 8). Can you consider doing that?

Comment 10 Frank Becker

2010-09-01 16:52:26 EDT

Created attachment 178004 [details]
commited patch V3

here is what I commit to HEAD.

Steffen is this what you requested in comment#9 ?

Comment 11 Frank Becker

2010-09-01 16:52:28 EDT

Created attachment 178005 [details]
mylyn/context/zip

Comment 12 Frank Becker

2010-09-01 16:54:49 EDT

Steffe,

if you think there is some open work please reopen this bug.

Comment 13 Steffen Pingel

2011-01-14 14:55:13 EST

*** Bug 334413 has been marked as a duplicate of this bug. ***