Skip to content

Added Queue column to RecurringJobsPage#1190

Open
Deantwo wants to merge 9 commits into
HangfireIO:mainfrom
Deantwo:patch-1
Open

Added Queue column to RecurringJobsPage#1190
Deantwo wants to merge 9 commits into
HangfireIO:mainfrom
Deantwo:patch-1

Conversation

@Deantwo
Copy link
Copy Markdown

@Deantwo Deantwo commented May 25, 2018

This is untested code, but should work in theory.

Added a Queue column to the list of RecurringJobs on the RecurringJobsPage of the dashboard.

Fixes #1189
Might help with issues like #1149 too

@codecov-io
Copy link
Copy Markdown

codecov-io commented May 25, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@1bfb550). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1190   +/-   ##
=========================================
  Coverage          ?   68.25%           
=========================================
  Files             ?      155           
  Lines             ?     5116           
  Branches          ?     1075           
=========================================
  Hits              ?     3492           
  Misses            ?     1473           
  Partials          ?      151

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 1bfb550...64d1866. Read the comment docs.

Never messed with a cshtml file before.
@pieceofsummer
Copy link
Copy Markdown
Contributor

Thanks for your PR! Would you mind to commit updated Strings.Designer.cs and also provide localized strings for other languages (you can simply copy them from corresponding QueuesPage_Table_Queue)?

@Deantwo
Copy link
Copy Markdown
Author

Deantwo commented Jun 7, 2018

I think that should do it.

@odinserj
Copy link
Copy Markdown
Member

@pieceofsummer, merging? For 1.6.20 (master branch) we should ensure that the dashboard page looks fine, for 1.7.0 (dev branch) we can postpone this task to test it later.

@pieceofsummer
Copy link
Copy Markdown
Contributor

@odinserj this shouldn't break anything, but it is not an urgent change.

We might first merge it into dev (and release the second beta), then merge into master if there are no layout issues because of increased content width.

@odinserj
Copy link
Copy Markdown
Member

@pieceofsummer, sounds good!

@Deantwo
Copy link
Copy Markdown
Author

Deantwo commented Jun 11, 2018

I don't have the time to add it in this pull request, but #1194 might be equally as easy to implement.

@pieceofsummer
Copy link
Copy Markdown
Contributor

@Deantwo unfortunately, it won’t be as easy. There’s no Queue field associated with the Processing state, so it would require either adding it, or extracting from the previous Enqueued state.

@Deantwo
Copy link
Copy Markdown
Author

Deantwo commented Jun 11, 2018

@pieceofsummer nevermind then. Issue just seemed very similar.

@pieceofsummer pieceofsummer changed the base branch from master to dev June 14, 2018 14:58
@pieceofsummer pieceofsummer changed the base branch from dev to master June 14, 2018 14:59
@Deantwo
Copy link
Copy Markdown
Author

Deantwo commented Jan 24, 2019

So, is this being merged? Or do I need to change anything?

@Deantwo Deantwo requested a review from odinserj February 25, 2020 09:58
@Deantwo
Copy link
Copy Markdown
Author

Deantwo commented Apr 16, 2020

What changed to make those tests fail?
This is a very simple little change, so getting it merged would be nice.

I was even missing this feature yesterday.

@Deantwo
Copy link
Copy Markdown
Author

Deantwo commented May 15, 2020

This isn't getting merged because it would make it more obvious there is a big issue with recurring jobs and queues?
Since I just ran head first into the #595 issue myself, and this minor change would make that a lot more obvious.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Recurring jobs on the dashboard don't show which queue it is using

4 participants