Closed Bug 747337 Opened 13 years ago Closed 13 years ago

convert blank fields to None/NULL when updating rules

Categories

(Release Engineering :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bhearsum, Assigned: edransch)

References

Details

Attachments

(1 file, 1 obsolete file)

I just used the new rules UI (/rules.html) to add a new rule. It succeeded, but the database rows ended up with empty strings for blank fields rather than NULLs.
Attached patch Change '' to NULL in rules forms (obsolete) (deleted) — Splinter Review
Attachment #617595 - Flags: review?(bhearsum)
The above attachment also changes the update_type field to a select field with the options 'major' and 'minor'. I can file a separate bug for this if need be.
Comment on attachment 617595 [details] [diff] [review] Change '' to NULL in rules forms Review of attachment 617595 [details] [diff] [review]: ----------------------------------------------------------------- Need to fix the valuelist/data stuff. Looks OK otherwise. ::: auslib/web/views/forms.py @@ +34,5 @@ > log.debug('JSONTextField: No value list, setting self.data to {}') > self.data = {} > > +class NullableTextField(TextField): > + """TextField that parses incoming data as JSON.""" docstring needs updating. @@ +37,5 @@ > +class NullableTextField(TextField): > + """TextField that parses incoming data as JSON.""" > + def process_formdata(self, valuelist): > + if valuelist and valuelist[0]: > + if valuelist[0] == '' or self.data == '': You shouldn't be checking self.data here - process_formdata is responsible for setting it based on the incoming data in 'valuelist'. @@ +44,5 @@ > + self.data = None > + else: > + log.debug('NullableTextField: No value list, setting self.data to None') > + self.data = None > + valuelist[0] = None Similarly, there's no need to change valuelist[0] at any point in this method.
Attachment #617595 - Flags: review?(bhearsum) → review-
Corrected the logic in process_formdata. I had misunderstood how it works.
Attachment #617595 - Attachment is obsolete: true
Attachment #617627 - Flags: review?(bhearsum)
Comment on attachment 617627 [details] [diff] [review] Updated patch - address feedback Review of attachment 617627 [details] [diff] [review]: ----------------------------------------------------------------- Jenkins run is here: https://jenkins.mozilla.org/job/Balrog/84/
Attachment #617627 - Flags: review?(bhearsum)
Attachment #617627 - Flags: review+
Attachment #617627 - Flags: checked-in+
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: