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)
Release Engineering
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: bhearsum, Assigned: edransch)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
bhearsum
:
review+
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•13 years ago
|
||
Attachment #617595 -
Flags: review?(bhearsum)
Assignee | ||
Comment 2•13 years ago
|
||
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.
Reporter | ||
Comment 3•13 years ago
|
||
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-
Assignee | ||
Comment 4•13 years ago
|
||
Corrected the logic in process_formdata. I had misunderstood how it works.
Attachment #617595 -
Attachment is obsolete: true
Attachment #617627 -
Flags: review?(bhearsum)
Reporter | ||
Comment 5•13 years ago
|
||
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+
Reporter | ||
Updated•13 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
Updated•7 years ago
|
Component: General Automation → General
You need to log in
before you can comment on or make changes to this bug.
Description
•