Closed Bug 574892 Opened 14 years ago Closed 14 years ago

Initial CC and Default QA Contact not set on bugs if the arguments are not passed to Bugzilla::Bug->create

Categories

(Bugzilla :: Creating/Changing Bugs, defect)

defect
Not set
major

Tracking

()

RESOLVED FIXED
Bugzilla 4.0

People

(Reporter: mkanat, Assigned: mkanat)

References

Details

Attachments

(1 file, 1 obsolete file)

The bug that created REQUIRED_FIELD_MAP and _required_create_fields handled most of the problems with defaulted fields in Bugzilla::Bug, but it missed a few. At the moment, at least CC and QA Contact need to also be defaulted.
Sigh. And groups.
Group: bugzilla-security
Attached patch v1 (obsolete) — Splinter Review
Assignee: create-and-change → mkanat
Status: NEW → ASSIGNED
Attachment #454210 - Flags: review?
Blocks: 574896
Comment on attachment 454210 [details] [diff] [review]
v1

>+their database defaults, or they are in a table different

they are _stored_ in a table ...

>+So it needs to be specified. The "QA Contact" field defaults to "NULL" 

This is a fragment. Either combine it into previous sentence or change "So" to "As a result," or something similar.

>+in the "bugs" table, but it defaults to the "intitial_qa_contact" of

s/intitial/initial/

>+the component. So it needs to be specified here.

Same comment as above.
Comment on attachment 454210 [details] [diff] [review]
v1

>+So it needs to be specified. The "QA Contact" field defaults to "NULL" 
>+in the "bugs" table, but it defaults to the "intitial_qa_contact" of
>+the component. So it needs to be specified here.

Also, this doesn't really make sense... The blahblah field defaults to blah in the blah, but it defaults to the blah of the blah. You're saying two different things but calling each one the default... There Can Only Be One.
target_milestone needs to be in there, too, because it defaults to defaultmilestone, not the database default.
(In reply to comment #1)
> Sigh. And groups.

I thought this was addressed by the other bug??
(In reply to comment #6)
> I thought this was addressed by the other bug??

  It was supposed to be, but ironically, it was one of the few fields that I missed, because we need this bit of extra framework.
Comment on attachment 454210 [details] [diff] [review]
v1

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

>+use constant EXTRA_REQUIRED_FIELDS => qw(cc qa_contact groups);

Hum, do you mean the caller has to specify these fields? If yes, then I disagree. A bug has defaults, and unless explicitly overriden, these defaults should be used (I hope mandatory groups cannot be overriden).

I want to test it before we release 3.7.2.
Attachment #454210 - Flags: review?(LpSolit)
(In reply to comment #8)
> Hum, do you mean the caller has to specify these fields?

  No, that was the whole point of the refactoring. No caller ever has to set any fields--if they have defaults, they will be set whether they're in the hash or not, now.
Attached patch v2Splinter Review
This adds target_milestone for Bugzilla::Bug, and clarifies the text in Bugzilla::Object.
Attachment #454210 - Attachment is obsolete: true
Attachment #455978 - Flags: review?(LpSolit)
Attachment #454210 - Flags: review?(LpSolit)
Attachment #454210 - Flags: review?
Comment on attachment 455978 [details] [diff] [review]
v2

r=LpSolit
Attachment #455978 - Flags: review?(LpSolit) → review+
Flags: approval+
Security advisory sent, unlocking bug.

Also, I checked this in as revision 7277 but apparently forgot to paste the commit message.
Group: bugzilla-security
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: