734395 - Reset data_version in Balrog admin forms on submit

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Erick Dransch [:edransch]]

If a submitted form modifies the databases, it will change the data_value for the corresponding row.
When this happens, the form's stored data_value should be updated to reflect the new value, otherwise the user will have to reload the page each time they update a row of the database.

Comment 1

[[e-mail] (:bhearsum)]

Attachment: make put/post/delete methods return new_data_version in their response; update form elements on success

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 609797 [details] [diff] [review]
make put/post/delete methods return new_data_version in their response; update form elements on success

Review of attachment 609797 [details] [diff] [review]:
-----------------------------------------------------------------

::: auslib/db.py
@@ +717,5 @@
>  
> +    def getPermission(self, username, permission, transaction=None):
> +        try:
> +            row = self.select(where=[self.username==username, self.permission==permission], transaction=transaction)[0]
> +            if row:

I'm curious why you wrote the code this way. Won't it fall straight through to the except if a non-empty array isn't returned by the select ? What does the 'if row' guard against ?

::: auslib/web/static/ausadmin.js
@@ +48,1 @@
>  

I see you have a test for adding a permission returning new_data_version, but does addNewPermission() do the right thing with it on success ?

Comment 3

[[e-mail] (:bhearsum)]

Comment on attachment 609797 [details] [diff] [review]
make put/post/delete methods return new_data_version in their response; update form elements on success

Review of attachment 609797 [details] [diff] [review]:
-----------------------------------------------------------------

::: auslib/db.py
@@ +717,5 @@
>  
> +    def getPermission(self, username, permission, transaction=None):
> +        try:
> +            row = self.select(where=[self.username==username, self.permission==permission], transaction=transaction)[0]
> +            if row:

Good point, I can't see a case where row would be empty.

::: auslib/web/static/ausadmin.js
@@ +48,1 @@
>  

It does, in that it fetches the HTML for the entire row again (so that we don't have to duplicate that structure in JS):
    ).success(function(data) {
        $.get(url, {'format': 'html'})
        .error(handleError
        ).success(function(data) {
            element.append(data);
        }); 
    }); 

So, it's not strictly necessary to return the data version for a new permission, I suppose.

Comment 4

[[e-mail] (:bhearsum)]

Comment on attachment 609797 [details] [diff] [review]
make put/post/delete methods return new_data_version in their response; update form elements on success

Landed, with the superfluous 'if' removed. Jenkins run is here: https://jenkins.mozilla.org/job/Balrog/70/