Maintain pagination when an user role changes #3120

ilausuch · 2020-05-27T14:07:05Z

https://progress.opensuse.org/issues/47210

Problem: In the user administration page. When a role is changed
the page is reloaded loosing the page in the paginator.

Solution: Programmatically summit the form to prevent the page
reload. This solves the problem

codecov · 2020-05-27T14:34:13Z

Codecov Report

Merging #3120 into master will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3120      +/-   ##
==========================================
+ Coverage   91.89%   91.92%   +0.03%     
==========================================
  Files         211      211              
  Lines       12921    12924       +3     
==========================================
+ Hits        11874    11881       +7     
+ Misses       1047     1043       -4

Impacted Files	Coverage Δ
lib/OpenQA/Worker/Job.pm	`75.51% <100.00%> (+0.59%)`	⬆️
lib/OpenQA/WebAPI/Plugin/Helpers.pm	`95.91% <0.00%> (+0.40%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cafc562...42e1242. Read the comment docs.

Martchus · 2020-05-27T14:55:09Z

assets/javascripts/admin_user.js

+                url: form.attr('action'),
+                data: jQuery.param(data),
+                error: function(err){
+                    rollback();


This doesn't work. The reason is likely that the rollback() function uses this but is not invoked with a this parameter.

Martchus · 2020-05-27T14:56:30Z

assets/javascripts/admin_user.js

+                }
+            });
+        } else
+            rollback();


Same as the previous comment regarding calling rollback(). Besides, please use braces here for symmetry.

Ahh a last change before commit, fixing this

Sorry, yes, this is working, but I could rewrite in a better way

I gave it a local test and it wasn't working (as well as the other rollback()).

Hope now works for you. Now this stores the form in a variable before to be used in the callbacks.

Martchus · 2020-05-27T14:57:01Z

assets/javascripts/admin_user.js

-            $(this).parent('form').submit();
-        } else {
+
+        function rollback(){


Please use a space like ) {.

Martchus · 2020-05-27T14:59:16Z

assets/javascripts/admin_user.js

+            var form = $(this).parent('form');
+            var data = form.serializeArray();
+
+            $.ajax({


Since the rollback() function relies on the class being set to default that class needs to be updated when this AJAX call succeeds.

this is solved now. I am using removeClass and addClass from jquery, but if is necessary could be replaced by native javascript operations

ilausuch · 2020-05-28T08:16:44Z

Additionally I added a small piece of html to show an beautiful red alert if something happens on updating the role.

ilausuch · 2020-05-28T08:18:41Z

Do you thing should be necessary expanse the test to include that the default class changes?

Martchus · 2020-05-28T09:48:45Z

templates/webapi/admin/user/index.html.ep

@@ -9,6 +9,14 @@
    <div class="col-sm-12">
        <h2><%= title %></h2>

+        <div id="error" class="alert alert-danger" role="alert" style="display:none">


The generic way this error is handled means that this is not necessarily an "Internal server error". Either distinguish the errors or keep it simple, e.g. "An error occurred when changing the user role.". I also wouldn't include the link to the bug tracker. After all the server might just be not reachable because the client's internet connection went off. And I would prefer to simply show a flash message using addFlash('danger', …); instead of creating the elements manually.

Good point. I didn't have notice of addFlash, very useful

Martchus · 2020-05-28T09:50:57Z

Do you thing should be necessary expanse the test to include that the default class changes?

Likely it isn't necessary to have automatic tests for every detail.

Martchus · 2020-05-28T10:50:39Z

assets/javascripts/admin_user.js

+
+            $.ajax({
+                type: 'POST',
+                url: form.attr('action')+'sddsds',


+'sddsds' ?

okurz · 2020-06-05T08:43:51Z

so one test failed "scheduler". That should be fixed meanwhile in master. We can retrigger the failing test which we did now or just rebase. I am not sure what changes @Martchus would still expect if any

https://progress.opensuse.org/issues/47210 Problem: In the user administration page. When a role is changed the page is reloaded loosing the page in the paginator. Solution: Programmatically summit the form to prevent the page reload. This solves the problem.

ilausuch · 2020-06-05T08:46:43Z

Re-based and pushed

Martchus

Seems to work now.

Martchus requested changes May 27, 2020

View reviewed changes

ilausuch force-pushed the users_mantain_page_in_pagination branch from 659895f to b37ec17 Compare May 28, 2020 08:14

Martchus requested changes May 28, 2020

View reviewed changes

ilausuch force-pushed the users_mantain_page_in_pagination branch from b37ec17 to 66779a9 Compare May 28, 2020 10:35

Martchus reviewed May 28, 2020

View reviewed changes

ilausuch force-pushed the users_mantain_page_in_pagination branch from 66779a9 to e940123 Compare May 28, 2020 12:12

okurz approved these changes May 29, 2020

View reviewed changes

ilausuch force-pushed the users_mantain_page_in_pagination branch from e940123 to 42e1242 Compare June 5, 2020 08:46

Martchus approved these changes Jun 5, 2020

View reviewed changes

kalikiana merged commit 8307215 into os-autoinst:master Jun 5, 2020

ilausuch deleted the users_mantain_page_in_pagination branch November 5, 2020 09:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Maintain pagination when an user role changes #3120

Maintain pagination when an user role changes #3120

ilausuch commented May 27, 2020

codecov bot commented May 27, 2020 •

edited

Martchus May 27, 2020 •

edited

Martchus May 27, 2020

ilausuch May 27, 2020 •

edited

ilausuch May 27, 2020

Martchus May 27, 2020

ilausuch May 28, 2020

Martchus May 27, 2020

Martchus May 27, 2020

ilausuch May 27, 2020 •

edited

ilausuch May 28, 2020

ilausuch commented May 28, 2020

ilausuch commented May 28, 2020

Martchus May 28, 2020

ilausuch May 28, 2020

Martchus commented May 28, 2020

Martchus May 28, 2020

ilausuch May 28, 2020

okurz commented Jun 5, 2020

ilausuch commented Jun 5, 2020

Martchus left a comment

Maintain pagination when an user role changes #3120

Maintain pagination when an user role changes #3120

Conversation

ilausuch commented May 27, 2020

codecov bot commented May 27, 2020 • edited

Codecov Report

Martchus May 27, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilausuch May 27, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilausuch May 27, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilausuch commented May 28, 2020

ilausuch commented May 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Martchus commented May 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

okurz commented Jun 5, 2020

ilausuch commented Jun 5, 2020

Martchus left a comment

Choose a reason for hiding this comment

codecov bot commented May 27, 2020 •

edited

Martchus May 27, 2020 •

edited

ilausuch May 27, 2020 •

edited

ilausuch May 27, 2020 •

edited