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

Hide re-enable and forgive buttons on success #2281

Merged
merged 1 commit into from
Nov 12, 2017

Conversation

daveFNbuck
Copy link
Contributor

Description

Make re-enable and forgive failure buttons disappear on server response. Also add the ability to hold alt with the re-enable to get an update like with forgive failures. This doesn't check for whether the server was successful in its response.

Motivation and Context

After fixing a bug or system issue that causes a class of jobs to fail, I'm often left with a lot of disabled jobs that need to be re-enabled. I usually click one button and then try to quickly click as many as I can before the refresh. This process is made much slower and less reliable by all the reloads. Some of my co-workers use the javascript console to ensure they can click every available button in time.

On the other hand, I'm not even sure if I successfully clicked on the forgive failures button when I want to make a job pending again. These buttons have very similar functions but wildly different behavior. In order to make them more consistent and more reasonable, we delete them to indicate receipt of the server response.

Have you tested this? If so, how?

I tried this out on my production server and it works fine.

After clicking re-enable, the whole page refreshes. After clicking
forgive failures, nothing happens. Both of these responses are a bit
extreme. To find a happier medium, this updates both buttons to simply
disappear after the response is received from the server. This makes it
easier to do multiple disables and allows for more confidence in
forgiving failures.

This also adds the ability to hold alt with the re-enable to get an
update like with forgive failures.
Copy link
Collaborator

@dlstadther dlstadther left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Tarrasch
Copy link
Contributor

LGTM

@Tarrasch Tarrasch merged commit 842ab3f into spotify:master Nov 12, 2017
This was referenced Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants