Fix processing job list #8323
Fix processing job list #8323
Conversation
…ditions for job description rendering
📝 WalkthroughWalkthroughThe pull request introduces improvements to job management in the frontend, focusing on two main files: Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
conf/application.conf
Outdated
| jobsEnabled = false | ||
| voxelyticsEnabled = false |
There was a problem hiding this comment.
remove before merging
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
frontend/javascripts/admin/api/jobs.ts (1)
Line range hint
46-50: Fix the sorting order in getJobs.The current sorting logic is incorrect. Using
>will sort in ascending order (oldest first), but the comment indicates "Newest jobs should be first".Apply this fix:
return ( jobs .map(transformBackendJobToAPIJob) // Newest jobs should be first - .sort((a: APIJob, b: APIJob) => a.createdAt > b.createdAt) + .sort((a: APIJob, b: APIJob) => (a.createdAt < b.createdAt ? 1 : -1)) );
🧹 Nitpick comments (2)
frontend/javascripts/admin/job/job_list_view.tsx (2)
160-167: LGTM! Clean extraction of link generation logic.The function effectively centralizes the dataset link generation logic with clear precedence rules.
Consider adding a type for the return value to make it explicit:
- function getLinkToDataset(job: APIJob) { + function getLinkToDataset(job: APIJob): string | null {
Line range hint
170-275: Consider adding error boundaries for link rendering.The code assumes that link generation and rendering will always succeed when
linkToDatasetis not null. Consider wrapping the link rendering in error boundaries to gracefully handle any runtime errors.Example implementation:
function SafeLink({ to, children }: { to: string; children: React.ReactNode }) { try { return <Link to={to}>{children}</Link>; } catch (error) { console.error('Error rendering link:', error); return <span>{children}</span>; } }Then use it in the render functions:
- <Link to={linkToDataset}>{job.datasetName}</Link> + <SafeLink to={linkToDataset}>{job.datasetName}</SafeLink>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/javascripts/admin/api/jobs.ts(1 hunks)frontend/javascripts/admin/job/job_list_view.tsx(6 hunks)
🔇 Additional comments (1)
frontend/javascripts/admin/api/jobs.ts (1)
24-25: Keep only the organizationId change.According to the PR objectives, only the
organizationIdfallback is needed. ThelayerNamechange appears to be unrelated to the current issue.
follow-up for #8300: sometimes, the fallback string was still shown as job description. this was due to unnecessary checks and changes in the backend.
URL of deployed dev instance (not really used for testing because jobs are disabled)
Steps to test:
(Please delete unneeded items, merge only when none are left open)