Last Comment Bug 782210 - If a custom field depends on a product, component or classification, the "mandatory" bit is ignored on bug creation
: If a custom field depends on a product, component or classification, the "man...
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Creating/Changing Bugs (show other bugs)
: 4.2.2
: All All
: -- normal (vote)
: Bugzilla 4.2
Assigned To: Pami Ketolainen
: default-qa
Mentors:
: 785746 817814 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-13 02:02 PDT by gidelson
Modified: 2013-05-09 16:38 PDT (History)
5 users (show)
LpSolit: approval+
LpSolit: approval4.4+
LpSolit: approval4.2+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
v1 (2.61 KB, patch)
2013-03-08 03:07 PST, Pami Ketolainen
no flags Details | Diff | Splinter Review
v2 (2.22 KB, patch)
2013-04-15 01:24 PDT, Pami Ketolainen
LpSolit: review+
Details | Diff | Splinter Review
v2 for 4.2 (2.09 KB, patch)
2013-04-15 23:55 PDT, Pami Ketolainen
LpSolit: review+
Details | Diff | Splinter Review

Description gidelson 2012-08-13 02:02:41 PDT
User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 5.1; Trident/4.0; .NET CLR 1.1.4322; .NET CLR 2.0.50727; .NET CLR 3.0.4506.2152; .NET CLR 3.5.30729; .NET4.0C; .NET4.0E)

Steps to reproduce:

Defines 5 custom fields of type large text box , that appear depending on classification field value.
all custom fields are mandatory.
In addition , define another custom field of type large text box , that is optional



Actual results:

Upon creating new bug , the user will get error if he didnt enter text of one of the first four mandatory custom fields. the system will NOT issue an error ,if the user forgot to enter text in the last custom field.

it seems that the last custom field mandatory bit is ignored.
if I make the 6th custom field as mandatory , the 5th field will behave normally (force update by user)


Expected results:

all custom fields should check the mandatoy bit.
Comment 1 Frédéric Buclin 2012-08-26 15:12:09 PDT
*** Bug 785746 has been marked as a duplicate of this bug. ***
Comment 2 gidelson 2013-02-11 02:46:40 PST
It happen also if the field configured to appear depending on product
Comment 3 Pami Ketolainen 2013-03-08 00:42:18 PST
I have a mandatory custom field that is visible only for certain products and I can confirm that the mandatory bit is ignored.

I think the reason might be that in Bug->run_create_validators() product and component in params are replaced with product_id and component_id before the _check_field_is_mandatory() is run. That ends up in ChoiceInterface->is_set_on_bug(), which always returns 0 as product is not defined in params hash passed initially to _check_field_is_mandatory()
Comment 4 Pami Ketolainen 2013-03-08 03:07:18 PST
Created attachment 722726 [details] [diff] [review]
v1

Here's a patch to check the mandatory fields before product and component are removed from the params.

For classification it is a bit more complex case as it's not a real field in bug, but instead a dynamic reference to product classification. I modified the _check_product() to include classification name in params when creating a bug. That feels a bit hackish, but I couldn't figure a better way to do it.
Comment 5 gidelson 2013-03-08 13:41:10 PST
Thanks.
Can you tell me how I insert those changes into Bugzilla files you changed?
by using patch? or some kind of diff editor?
Comment 6 Frédéric Buclin 2013-03-09 04:27:14 PST
Please request review for your patch to put it into reviewers' radar.
Comment 7 Frédéric Buclin 2013-04-13 10:32:48 PDT
I can reproduce.
Comment 8 Frédéric Buclin 2013-04-13 10:45:52 PDT
Comment on attachment 722726 [details] [diff] [review]
v1

>=== modified file 'Bugzilla/Bug.pm'

> sub _check_product {

>+    $params->{classification} = $product->classification->name;

Your patch looks good, but I would prefer this line to go into run_create_validators() itself, because $params doesn't exist in _check_product() when updating bugs and so this code doesn't need to be triggered in this case.
Comment 9 Pami Ketolainen 2013-04-15 01:24:29 PDT
Created attachment 737383 [details] [diff] [review]
v2

Updated the patch. Adding the classification in run_create_validators() also makes it more clear why it's done
Comment 10 Frédéric Buclin 2013-04-15 12:26:29 PDT
Comment on attachment 737383 [details] [diff] [review]
v2

Thanks for the patch! :) r=LpSolit
Comment 11 Frédéric Buclin 2013-04-15 14:22:03 PDT
The patch applies cleanly to 4.4 too, but doesn't apply cleanly to the 4.2 branch. Pami, do you want to backport it or should I?
Comment 12 Pami Ketolainen 2013-04-15 23:55:50 PDT
Created attachment 737844 [details] [diff] [review]
v2 for 4.2

Modified the patch for 4.2
Comment 13 Frédéric Buclin 2013-04-16 03:10:46 PDT
Comment on attachment 737844 [details] [diff] [review]
v2 for 4.2

r=LpSolit
Comment 14 Frédéric Buclin 2013-04-16 03:14:56 PDT
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Bug.pm
modified Bugzilla/Field/ChoiceInterface.pm
Committed revision 8611.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.4/
modified Bugzilla/Bug.pm
modified Bugzilla/Field/ChoiceInterface.pm
Committed revision 8544.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified Bugzilla/Bug.pm
modified Bugzilla/Field/ChoiceInterface.pm
Committed revision 8205.
Comment 15 Frédéric Buclin 2013-05-09 16:38:25 PDT
*** Bug 817814 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.