1363058 - don't send empty strings for null values when a field's value is removed

Status: RESOLVED FIXED

(Release Engineering Graveyard :: Applications: Balrog (frontend), enhancement, P3)

Description

[[e-mail] (:bhearsum)]

We send empty strings for null values right now. This is wrong! Null values should be sent as null so that they show up as None on the backend side.

Comment 1

[Harkishen H [:harikishenh]]

I'm a new contributor to balrog. I would love to take this up and work on it.

Comment 2

[Ninad Bhat[:ninad101]]

Hi Harkishen,

Thanks for the interset. You can start by reading http://mozilla-balrog.readthedocs.io/en/latest/contribute.html. 
If you have any problem, you can comment here or ask at #balrog on IRC at irc.mozilla.org.

Comment 3

[Harkishen H [:harikishenh]]

I have set up the environment. I would like to know where to start with this bug ?

Comment 4

[Ninad Bhat[:ninad101]]

(In reply to Harkishen H [:harikishenh] from comment #3)
> I have set up the environment. I would like to know where to start with this
> bug ?

bhearsum will be able to help you with this. I will needinfo him.

Comment 5

[Harkishen H [:harikishenh]]

anything ?

Comment 6

[Ninad Bhat[:ninad101]]

Hi,
It might take some time to get the reply. I saw that you wanted to work on another bug(https://bugzilla.mozilla.org/show_bug.cgi?id=1346212) too. You can start working on it.

Comment 7

[[e-mail] (:bhearsum)]

(In reply to Harkishen H [:harikishenh] from comment #5)
> anything ?

Hi Harkishen, sorry for the delay in response. I'm actually not sure if this is a bug anymore...I'm having trouble reproducing it. Ashish, do you remember where you were seeing this?

Comment 8

[A.S.]

we encountered this when attempting to create or edit a rule. Here's the comment with the logs : https://github.com/mozilla/balrog/pull/312#issuecomment-301002486 . To reproduce it, try typing into any field and then delete the field value so that the form will send a blank string '' in the request. Example: In above logs 'product' was getting passed as blank. 

In order to fix this bug from backend. I added an extra check for blank values here : https://github.com/mozilla/balrog/blob/bae00cbcec1357c32b09d59fb51f64e6ecb3d5d9/auslib/web/admin/views/rules.py#L57

Comment 9

[[e-mail] (:bhearsum)]

Thanks Ashish.

So, probably the best first step here is to get the development environment running locally (http://mozilla-balrog.readthedocs.io/en/latest/contribute.html#usage) and then reproducing the bug by:
1) Browsing to http://localhost:8080/rules
2) Opening the Developer Toolbar
3) Clicking "Update" for a rule
4) Deleting the contents of the "product" field
5) Submitting the update
6) Look at the request. Observe that the "product" field's value is sent as an empty string, where other empty fields are sent as null.

What we're looking for in this bug is to send any empty field as null, even if it previously had a value in it. The fix for this is most likely in the controllers or services from https://github.com/mozilla/balrog/tree/master/ui/app, and needs to be fixed for all types of objects (Rules, Releases, Permissions, Required Signoffs, and their associated Scheduled Changes).

Comment 10

[Harkishen H [:harikishenh]]

It seems to be stuck when i submit the update
http://picpaste.com/Screen_Shot_2017-05-24_at_4.11.32_PM-TZR3NbBp.png
look at it

Comment 11

[Harkishen H [:harikishenh]]

sorry about that i tried schedule update instead of update

Comment 12

[Harkishen H [:harikishenh]]

i was considering adding something like this to rules_service.js

      for (var i in data) {
        if (data.hasOwnProperty(i)) {
          if (data[i] === "") {
              data[i] = null;
          }
        }
      }
is that fine ???

Comment 13

[Harkishen H [:harikishenh]]

similarly for the other objects services

Comment 14

[Ninad Bhat[:ninad101]]

Harkishen it would be better if you made the changes and send a PR. It is easier to review and reply there.

Comment 15

[Harkishen H [:harikishenh]]

ohk

Comment 16

[[github robot]]

Commit pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/533415190a3e394435498ac7afd4b7ae6434bd73
bug 1363058: don't send empty strings for null values when a field's value is removed (#321). r=bhearsum

Comment 17

[[e-mail] (:bhearsum)]

Thanks for your contribution here Harkishen!