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

RESOLVED FIXED in Bugzilla 4.2

Status

()

Bugzilla
Creating/Changing Bugs
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: gidelson, Assigned: Pami Ketolainen)

Tracking

4.2.2
Bugzilla 4.2
Bug Flags:
approval +
approval4.4 +
approval4.2 +

Details

Attachments

(2 attachments, 1 obsolete attachment)

v2
2.22 KB, patch
Frédéric Buclin
: review+
Details | Diff | Splinter Review
2.09 KB, patch
Frédéric Buclin
: review+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
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.

Updated

5 years ago
Duplicate of this bug: 785746
(Reporter)

Comment 2

5 years ago
It happen also if the field configured to appear depending on product
(Assignee)

Comment 3

5 years ago
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()
(Assignee)

Comment 4

5 years ago
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.
(Reporter)

Comment 5

5 years ago
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

5 years ago
Please request review for your patch to put it into reviewers' radar.
(Assignee)

Updated

5 years ago
Attachment #722726 - Flags: review?(LpSolit)

Comment 7

4 years ago
I can reproduce.
Assignee: administration → pami.ketolainen
Status: UNCONFIRMED → ASSIGNED
Component: Administration → Creating/Changing Bugs
Ever confirmed: true
Summary: Custom field "mandatory" bit is ignored → If a custom field depends on a product, component or classification, the "mandatory" bit is ignored on bug creation
Target Milestone: --- → Bugzilla 4.2

Comment 8

4 years ago
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.
(Assignee)

Comment 9

4 years ago
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
Attachment #722726 - Attachment is obsolete: true
Attachment #722726 - Flags: review?(LpSolit)
Attachment #737383 - Flags: review?(LpSolit)

Comment 10

4 years ago
Comment on attachment 737383 [details] [diff] [review]
v2

Thanks for the patch! :) r=LpSolit
Attachment #737383 - Flags: review?(LpSolit) → review+

Updated

4 years ago
Flags: approval4.4+
Flags: approval4.2+
Flags: approval+

Comment 11

4 years ago
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?
(Assignee)

Comment 12

4 years ago
Created attachment 737844 [details] [diff] [review]
v2 for 4.2

Modified the patch for 4.2

Comment 13

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

r=LpSolit
Attachment #737844 - Flags: review+

Comment 14

4 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Duplicate of this bug: 817814
You need to log in before you can comment on or make changes to this bug.