747337 - convert blank fields to None/NULL when updating rules

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[[e-mail] (:bhearsum)]

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.

Comment 1

[Erick Dransch [:edransch]]

Attachment: Change '' to NULL in rules forms

Comment 2

[Erick Dransch [:edransch]]

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 3

[[e-mail] (:bhearsum)]

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.

Comment 4

[Erick Dransch [:edransch]]

Attachment: Updated patch - address feedback

Corrected the logic in process_formdata. I had misunderstood how it works.

Comment 5

[[e-mail] (: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/