Skip to content

Add "Delete All" button to Retries in Web UI#540

Merged
jc00ke merged 1 commit intosidekiq:masterfrom
brandonhilkert:master
Nov 26, 2012
Merged

Add "Delete All" button to Retries in Web UI#540
jc00ke merged 1 commit intosidekiq:masterfrom
brandonhilkert:master

Conversation

@brandonhilkert
Copy link
Collaborator

Added "Delete All" button that leverages the API to clear jobs.

dl

dl

dl

@jc00ke
Copy link
Contributor

jc00ke commented Nov 26, 2012

Nice! I'd prefer to see the button down by Retry Now and Delete, and I'd also prefer not to see the button if there are no jobs. Thoughts?

@bensie
Copy link
Contributor

bensie commented Nov 26, 2012

👍 How about a "Retry All" as well down there?

@brandonhilkert
Copy link
Collaborator Author

@jc00ke The position you mentioned was my first thought, but both of those buttons are controlled by the checkboxes, and something told me to put it somewhere else so that "Delete All" wasn't misinterpreted for "Delete All the ones on this page"? Does that make sense? My next thought was to maybe put it bottom left. Would that be better?

Agreed on not showing if there aren't jobs.

@jc00ke
Copy link
Contributor

jc00ke commented Nov 26, 2012

Maybe put buttons that are toggled by the checkboxes to the left (under the checkboxes) and Delete All/Retry All to the right?

@brandonhilkert
Copy link
Collaborator Author

@jc00ke Pushed.

dl

Is "Retry All" something you'd like me to add as well?

@jc00ke
Copy link
Contributor

jc00ke commented Nov 26, 2012

@brandonhilkert re: Retry All @bensie had mentioned it, and if it's easy to add, might as well do it now.

Also, does that screencap reflect the changes? I was thinking...

Retries
__________________________________________________
|[]                                              |
|[]                                              |
|[]                                              |
|________________________________________________|
Retry Now   Delete                      Delete All

That way the buttons that are activated by the checkboxes are under the checkboxes.

@brandonhilkert
Copy link
Collaborator Author

Let me take a quick look at the Retry All.

re: pic - exactly. New screenshot is attached above.

@brandonhilkert
Copy link
Collaborator Author

Should be all set.

dl

@brandonhilkert
Copy link
Collaborator Author

If this looks good, I can squash the commits if necessary. Let me know.

@bensie
Copy link
Contributor

bensie commented Nov 26, 2012

Looks awesome.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If there are 1000 retries, this will perform 1000 round trips. Just clear the sorted set in one go. I'd suggest adding a clear method to the API objects.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mperham I might be missing something, but is there a good way to clear a sorted set rather than delete the key entirely?

@jc00ke
Copy link
Contributor

jc00ke commented Nov 26, 2012

Yeah, looks great. Once @mperham's suggestion about the clear method is implemented please squash then we'll merge.

@fabianoalmeida
Copy link

I'd like to suggest a little change... Why don't put Delete and Retry Now for each line of table?

For example, create a new column called Actions and put these two buttons there. Using this way, we won't need the column with checkboxes.

My suggestion makes more objective the actions. What do you think? Am I wrong with my point? ☺️

@brandonhilkert
Copy link
Collaborator Author

@fabianoalmeida You then lose the ability to delete, say 20, with a single button click. I see the benefit for a single job, but beyond that, the actions seem tedious.

@brandonhilkert
Copy link
Collaborator Author

@mperham How's that?

@brandonhilkert
Copy link
Collaborator Author

I also have a wiki page all setup for the API once this is merged.

@mperham
Copy link
Collaborator

mperham commented Nov 26, 2012

Looks great, thanks!

@mperham
Copy link
Collaborator

mperham commented Nov 26, 2012

Oh, changelog update?

@geomic
Copy link

geomic commented Nov 26, 2012

This interface looks incredible @brandonhilkert - I personally can't wait till it makes it to master.

@brandonhilkert
Copy link
Collaborator Author

Updated changelog and rebased.

jc00ke added a commit that referenced this pull request Nov 26, 2012
Add "Delete All" button to Retries in Web UI
@jc00ke jc00ke merged commit 70f2965 into sidekiq:master Nov 26, 2012
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.

6 participants