Skip to content

fix(dashboard): Format job queue durations as human-readable h/m/s#4635

Open
biggamesmallworld wants to merge 1 commit intomasterfrom
fix/format-job-queue-durations
Open

fix(dashboard): Format job queue durations as human-readable h/m/s#4635
biggamesmallworld wants to merge 1 commit intomasterfrom
fix/format-job-queue-durations

Conversation

@biggamesmallworld
Copy link
Copy Markdown
Collaborator

@biggamesmallworld biggamesmallworld commented Apr 8, 2026

Description

Display job queue durations in a human-readable format (e.g. 2h 5m 3s, 45s, < 1s) instead of raw milliseconds (e.g. 125000ms).

Fixes OSS-471

Breaking changes

None.

Screenshots

Screenshot 2026-04-08 at 16 28 15

Checklist

📌 Always:

  • I have set a clear title
  • My PR is small and contains a single feature
  • I have checked my own PR

👍 Most of the time:

  • I have added or updated test cases
  • I have updated the README if needed

Display job queue durations in a human-readable format (e.g. "2h 5m 3s")
instead of raw milliseconds.

Fixes OSS-471

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 8, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
vendure-storybook Ready Ready Preview, Comment Apr 8, 2026 2:03pm

Request Review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 8, 2026

📝 Walkthrough

Walkthrough

Added a formatDurationMs(ms) helper function to convert millisecond durations into human-readable format (e.g., "< 1s", "Xh Ym Zs"). Updated the duration column cell rendering to apply this formatter to row.original.duration values, replacing the previous raw millisecond display. The null handling logic remains unchanged.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(dashboard): Format job queue durations as human-readable h/m/s' clearly and specifically describes the main change: converting millisecond durations to human-readable format in the dashboard.
Description check ✅ Passed The pull request description follows the template structure with all required sections completed: a clear description of changes, explicit statement of no breaking changes, a screenshot demonstrating the improvement, and a completed checklist.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/format-job-queue-durations

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 8, 2026

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 8, 2026

Dashboard Preview: https://admin-dashboard-bf5lh230f-vendure.vercel.app

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/dashboard/src/app/routes/_authenticated/_system/job-queue.tsx`:
- Around line 217-222: The current truthy check in the duration cell (cell: ({
row }) => { return row.original.duration ?
formatDurationMs(row.original.duration) : null; }) will hide zero durations;
replace the truthy check with an explicit null/undefined check (e.g.,
row.original.duration != null) so that 0 is treated as a valid duration and
passed to formatDurationMs, while null/undefined still yields null.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ae5ac506-564f-4d77-a4a2-66606492a584

📥 Commits

Reviewing files that changed from the base of the PR and between d1441ee and 29e73b2.

📒 Files selected for processing (1)
  • packages/dashboard/src/app/routes/_authenticated/_system/job-queue.tsx

Comment on lines 217 to 222
duration: {
cell: ({ row }) => {
return row.original.duration ? `${row.original.duration}ms` : null;
return row.original.duration
? formatDurationMs(row.original.duration)
: null;
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Truthy check would hide duration = 0 instead of showing < 1s.

The condition row.original.duration ? ... treats 0 as falsy. If a job completes instantly (duration of 0ms), this would render nothing instead of < 1s. Use an explicit null check to distinguish between "no duration yet" and "zero duration".

Proposed fix
 duration: {
     cell: ({ row }) => {
-        return row.original.duration
+        return row.original.duration != null
             ? formatDurationMs(row.original.duration)
             : null;
     },
 },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
duration: {
cell: ({ row }) => {
return row.original.duration ? `${row.original.duration}ms` : null;
return row.original.duration
? formatDurationMs(row.original.duration)
: null;
},
duration: {
cell: ({ row }) => {
return row.original.duration != null
? formatDurationMs(row.original.duration)
: null;
},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/dashboard/src/app/routes/_authenticated/_system/job-queue.tsx`
around lines 217 - 222, The current truthy check in the duration cell (cell: ({
row }) => { return row.original.duration ?
formatDurationMs(row.original.duration) : null; }) will hide zero durations;
replace the truthy check with an explicit null/undefined check (e.g.,
row.original.duration != null) so that 0 is treated as a valid duration and
passed to formatDurationMs, while null/undefined still yields null.

@Ryrahul
Copy link
Copy Markdown
Contributor

Ryrahul commented Apr 8, 2026

@biggamesmallworld Nice improvement ,the human-readable duration makes the dashboard much easier to scan 👍

I actually have an open PR that also formats durations (and adds bulk actions support)

Would you mind taking a quick look when you get a chance?

If everything else looks good from your side, I’m happy to remove the duration formatting part from mine so we can avoid overlap/conflicts as i was about to solve existing conflict there and just keep the bulk action there.

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.

2 participants