Bug 322680

Comment 1 Charley Wang 2010-08-13 16:22:15 EDT

Created attachment 176586 [details]
Do not try to submit blank values in new tasks

Resolves issue when Bugzilla installation:

1. Has custom fields
2. Does not display custom field options in config.cgi
3. Allows a custom field to be set on task creation

Currently, under those conditions, Mylyn will fail with 'Need to set Custom Field.' The patch will prevent the Mylyn post method from trying to send a custom field with no input, which avoids this 

The test suite runs without failures with this patch applied. When running AllBugzillaTests, I do get strange messages on the console about IncompatibleClassChange and a missing bugs36 repository, I do not believe they are related to this change.

Comment 2 Charley Wang 2010-08-13 16:22:17 EDT

Created attachment 176587 [details]
mylyn/context/zip

Comment 3 Frank Becker 2010-08-14 11:44:03 EDT

(In reply to comment #0)
> Problem:
> Some Bugzilla installations may not have custom field information available in
> their config.cgi.
> 
> This is problematic, because on task creation Mylyn attempts to send every
> available custom field as a parameter with a blank value. Bugzilla treats these
> blank parameters as invalid options, and task creation fails.
> 
Charley,

I did not see a way for Mylyn to have an BugzillaCustomField which is not in the config.cgi.
Can you please give some more information.

Only BugzillaCustomField with isEnterBug() == true are included in the new TaskData.

Comment 4 Frank Becker 2010-08-14 11:44:05 EDT

Created attachment 176600 [details]
mylyn/context/zip

Comment 5 Charley Wang 2010-08-19 13:35:28 EDT

It is possible to achieve this by commenting/removing two lines from config.cgi:

# $vars->{'custom_fields'} =
#    [ grep {$_->is_select} Bugzilla->active_custom_fields ];

This will cause config.cgi to not return custom_fields information, which causes the custom fields to be sent with blank values on task creation, which causes task creation to fail.

I believe in the case of the particular Bugzilla installation I was working with, the custom fields information may have been omitted to save space.

This is a pretty rare occurrence, I think, but one that breaks a key Mylyn feature if it happens. The proposed fix should address this case without affecting the other cases as far as I could tell from the test suites and from manual testing.

Comment 6 Frank Becker 2010-08-21 16:23:39 EDT

(In reply to comment #5)
> It is possible to achieve this by commenting/removing two lines from config.cgi:
> 
> # $vars->{'custom_fields'} =
> #    [ grep {$_->is_select} Bugzilla->active_custom_fields ];
> 
> This will cause config.cgi to not return custom_fields information, which causes
> the custom fields to be sent with blank values on task creation, which causes
> task creation to fail.
> 
This is only true until you do an update of the repository configuration. After the first update all should be fine.

If not then please reopen this bug.

Comment 7 Frank Becker 2010-08-21 16:23:42 EDT

Created attachment 177164 [details]
mylyn/context/zip

Comment 8 Charley Wang 2010-08-23 11:56:02 EDT

Hi again,

Sorry to reopen this, but I tried to update the repository configuration, and this problem is currently persistent with the Fedora/Red Hat Bugzilla:

https://bugzilla.redhat.com/

More details:
The failure is persistent when the custom field is a Drop Down. 

Steps to Reproduce:

1. Create new Drop Down custom field
2. Set Drop Down custom field to be available on bug creation
3. Remove custom fields information from config.cgi
4. Update repository, attempt to  create a bug with Mylyn

This should fail because the new Drop Down field was not specified. Because no custom fields information is available in config.cgi, there is no way to specify a value for the Drop Down field, which means I can't submit new tasks to this Bugzilla installation.

With the proposed patch, the task submission should go through (Mylyn will skip the custom Drop Down field altogether if it does not have any valid options).

A more ideal solution would involve allowing the user to type into a custom field if no options are available, but this patch will at least allow task submission to work.

Thanks for your feedback and attention! :)

Comment 9 Frank Becker 2010-08-23 14:24:35 EDT

(In reply to comment #5)
> It is possible to achieve this by commenting/removing two lines from
> config.cgi:
> 
> # $vars->{'custom_fields'} =
> #    [ grep {$_->is_select} Bugzilla->active_custom_fields ];
> 
> This will cause config.cgi to not return custom_fields information, which
> causes the custom fields to be sent with blank values on task creation, which
> causes task creation to fail.
> 

The two commented out lines only remove the list of legal values for an custom field. The list of fields contains the real definition of the select custom fields like all other custom field types.

For me this is nothing to fix within mylyn. I think you have to change the config.cgi to remove the definition of the custom fields of type drop down and multi select. 

Thoughts?

Comment 10 Charley Wang 2010-08-23 14:38:06 EDT

Yes, I'm sorry for not being clear -- the bug is not caused by the definition of the custom field being missing, it is caused by the list of legal values being missing.

Mylyn sees that the custom field can be set on bug creation, so it tries to send the custom field on bug creation. Normally, if the list of legal values is available, Mylyn will send the first value in the list as the default. However, since the list is not available for that field, Mylyn instead sends an empty string which is rejected by the server as an invalid value.

e.g. Mylyn sees cf_cust_facing and attempts to send:
/post.cgi?...&cf_cust_facing=&....

This prompts the server to complain that an invalid value was selected for cf_cust_facing.

It would be nice if, instead of trying to set the field to an empty string, Mylyn did not try to set the field at all.

Comment 11 Frank Becker 2010-08-23 14:57:42 EDT

(In reply to comment #10)
> Yes, I'm sorry for not being clear -- the bug is not caused by the definition
> of the custom field being missing, it is caused by the list of legal values
> being missing.
> 
> Mylyn sees that the custom field can be set on bug creation, so it tries to
> send the custom field on bug creation. Normally, if the list of legal values is
> available, Mylyn will send the first value in the list as the default. However,
> since the list is not available for that field, Mylyn instead sends an empty
> string which is rejected by the server as an invalid value.
> 
> e.g. Mylyn sees cf_cust_facing and attempts to send:
> /post.cgi?...&cf_cust_facing=&....
> 
> This prompts the server to complain that an invalid value was selected for
> cf_cust_facing.
> 
> It would be nice if, instead of trying to set the field to an empty string,
> Mylyn did not try to set the field at all.

No I think this is the wrong way. Why did you only remove the list of legal values and not the whole definition of the custom filed. For me it makes now sense to have an custom field with an empty list.

Comment 12 Charley Wang 2010-08-23 15:10:21 EDT

(In reply to comment #11)
> No I think this is the wrong way. Why did you only remove the list of legal
> values and not the whole definition of the custom filed. For me it makes now
> sense to have an custom field with an empty list.

Sorry for this weird setup, I agree that this does not make sense, but that is how the Red Hat/Fedora Bugzilla is currently set up. I am not sure why the list of legal values was removed. I believe it may have to do with keeping the configuration file size manageable (it is currently at 25 MB without the legal field values).

It would be really great if Mylyn would accept the patch -- the patch won't affect Bugzillas with a normal setup, and the automated tests run with the patch applied. But I understand if you don't think this configuration should be supported. 

I have been trying to work with the Bugzilla administrators as well to get the legal field values added back to the configuration file, I do not know if they will.

Thank you again for taking the time to look into this issue.

-Charley

Comment 13 Frank Becker 2010-08-23 16:47:41 EDT

Created attachment 177266 [details]
patch

Charley,

I think that is a much better way for a fix.

Thoughts?

Comment 14 Frank Becker 2010-08-23 16:47:44 EDT

Created attachment 177267 [details]
mylyn/context/zip

Comment 15 Charley Wang 2010-08-23 16:53:11 EDT

I agree! Your patch works great. I can confirm that your patch fixes the problem for the Red Hat/Fedora Bugzilla.

Thank you very much!

Comment 16 Frank Becker 2010-08-23 17:01:12 EDT

(In reply to comment #15)
> I agree! Your patch works great. I can confirm that your patch fixes the problem
> for the Red Hat/Fedora Bugzilla.
> 
> Thank you very much!

Patch is now in HEAD.