782210 - If a custom field depends on a product, component or classification, the "mandatory" bit is ignored on bug creation

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[gidelson]

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 2

[gidelson]

It happen also if the field configured to appear depending on product

Comment 3

[Pami Ketolainen]

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]

Attachment: 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]

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]

Please request review for your patch to put it into reviewers' radar.

Comment 7

[Frédéric Buclin]

I can reproduce.

Comment 8

[Frédéric Buclin]

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]

Attachment: 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]

Comment on attachment 737383 [details] [diff] [review]
v2

Thanks for the patch! :) r=LpSolit

Comment 11

[Frédéric Buclin]

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]

Attachment: v2 for 4.2

Modified the patch for 4.2

Comment 13

[Frédéric Buclin]

Comment on attachment 737844 [details] [diff] [review]
v2 for 4.2

r=LpSolit

Comment 14

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Bug.pm
modified Bugzilla/Field/ChoiceInterface.pm
Committed revision 8611.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.4/
modified Bugzilla/Bug.pm
modified Bugzilla/Field/ChoiceInterface.pm
Committed revision 8544.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.2/
modified Bugzilla/Bug.pm
modified Bugzilla/Field/ChoiceInterface.pm
Committed revision 8205.