Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

users: actions fail due to unlocking errors when clicking too fast #9558

Closed
martinpitt opened this issue Jul 2, 2018 · 7 comments
Closed
Labels
good-first-issue Appropriate for new contributors

Comments

@martinpitt
Copy link
Member

martinpitt commented Jul 2, 2018

The users page does not serialize or retry actions when doing lots of things in quick succession. This particularly affects our tests, check-accounts often fails with various errors. The associated screen shots say why:

screenshot

On other pages our approach is generally that we disable UI actions while an activity runs. This provides proper serialization and should work here as well.

@martinpitt
Copy link
Member Author

@stefwalter : FYI, this is the issue we talked about last week. I'll see to talk someone into fixing this :)

@martinpitt martinpitt added the good-first-issue Appropriate for new contributors label Jul 2, 2018
@as-rawat
Copy link

as-rawat commented Jul 3, 2018

I'd like to fix it! I don't even know where to start... It'll be great if you could provide some pointers 👶

@martinpitt
Copy link
Member Author

@as-rawat : Nice! It shouldn't be too difficult actually. If you want to give it a stab, here are some notes which hopefully help. Also, if you want to work on this, I strongly recommend reading Working on your local machine so that you can develop on Cockpit with fast iteration cycles.

For an old jquery/plain JavaScript page like users, disabling/enabling an element happens through the disabled property; the users page code already does that plenty of times, like this:

$('#my_element_id').prop('disabled', true);

So the exercise is to change change_role(), change_real_name() and all other functions to disable all UI elements before the cockpit.spawn (which calls things like passwd) and re-enable them again after spawn finishes.

Of course this should not disable every button/link individually, that would be laborius, error prone, and hard to maintain. It should work to disable a parent element which includes them all. My first guess would be #account-details.

It could also be that the tests need updating, to wait until elements become enabled again. (I'm happy to help with that).

@as-rawat
Copy link

as-rawat commented Jul 4, 2018

Thanks for pointers! I think, I'm not getting a perfect picture of what's happening here... but I think...
The bug is:
-users is able to trigger activities even when an activity is already running which affects tests and fails.
What I have to do is:
-Install cockpit
-When a user starts an activity(cockpit.swamp) disable everything on the page(DOM or a parent element) till its completed and then re-enable it.

Please correct me! 😕

@martinpitt
Copy link
Member Author

@as-rawat : You've got it right. 👍

@martinpitt
Copy link
Member Author

@as-rawat: Do you still want to work on this? If not, I'd hand this to someone else, as it's one of our most annoying causes of test flakes.

@as-rawat
Copy link

@martinpitt Sorry for not giving any details. I would like to work on it but wasn't able to cuz I was giving few interviews and searching for the jobs.

croissanne added a commit to croissanne/cockpit that referenced this issue Sep 12, 2018
Where dialog pop up, input elements are not disabled, since tests can
wait for those to pop down.

Fixes cockpit-project#9558
Closes cockpit-project#9979
martinpitt pushed a commit that referenced this issue Sep 13, 2018
Where dialog pop up, input elements are not disabled, since tests can
wait for those to pop down.

Fixes #9558
Closes #9979
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue Appropriate for new contributors
Projects
None yet
Development

No branches or pull requests

2 participants