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

Add run details pop out page #1335

Merged
merged 3 commits into from
May 10, 2024
Merged

Add run details pop out page #1335

merged 3 commits into from
May 10, 2024

Conversation

goodoldneon
Copy link
Contributor

⚠️ Depends on https://github.com/inngest/monorepo/pull/2614

Description

Add a new /env/production/runs/[runID] route that shows the new run details. This will be the "pop out" page where users can look at run details outside of the run list.

Testing

image

Type of change (choose one)

  • Chore (refactors, upgrades, etc.)
  • Bug fix (non-breaking change that fixes an issue)
  • Security fix (non-breaking change that fixes a potential vulnerability)
  • Docs
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality not to work as expected)

Checklist

  • I've linked any associated issues to this PR.
  • I've tested my own changes.

Check our Pull Request Guidelines

Copy link

vercel bot commented May 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 9, 2024 6:39pm

Comment on lines +97 to +104
if (isFunctionRunStatus(trace.status)) {
if (trace.status === 'CANCELLED') {
text = 'Cancelled';
} else if (trace.status === 'COMPLETED') {
text = 'Completed';
} else if (trace.status === 'FAILED') {
text = 'Failed';
}
Copy link
Member

Choose a reason for hiding this comment

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

suggestion (non-blocking): Since we're importing a function to check the type then use a switch-like statement to return the text, we could also just put this entire thing into a util fn if we use it in multiple places like: getStatusText(trace.status). Also, it would be ideal to check equality against constants and avoid string matching.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where's a good spot for that? ui/packages/components/src/types/functionRun.ts doesn't seem right since the user-facing status text is copy and not a type

Comment on lines +41 to +45
ended: toMaybeDate(trace.endedAt)?.getTime() ?? null,
max: maxTime.getTime(),
min: minTime.getTime(),
queued: new Date(trace.queuedAt).getTime(),
started: toMaybeDate(trace.startedAt)?.getTime() ?? null,
Copy link
Member

Choose a reason for hiding this comment

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

Much cleaner!

@goodoldneon goodoldneon changed the title Run details gql Add run details pop out page May 9, 2024
@goodoldneon goodoldneon merged commit 7a4de9f into main May 10, 2024
13 checks passed
@goodoldneon goodoldneon deleted the run-details-gql branch May 10, 2024 23:01
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.

None yet

2 participants