Skip to content

chore: test dashboard code change (DO NOT MERGE. DELETE AFTER ENG-2151)#4246

Closed
mcstepp wants to merge 4 commits intomainfrom
test/dashboard-code-change
Closed

chore: test dashboard code change (DO NOT MERGE. DELETE AFTER ENG-2151)#4246
mcstepp wants to merge 4 commits intomainfrom
test/dashboard-code-change

Conversation

@mcstepp
Copy link
Collaborator

@mcstepp mcstepp commented Nov 5, 2025

What does this PR do?

Fixes # (issue)

If there is not an issue for this, please create one first. This is used to tracking purposes and also helps us understand why this PR exists

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • Test A
  • Test B

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Ran make fmt on /go directory
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Unkey Docs if changes were necessary

@linear
Copy link

linear bot commented Nov 5, 2025

@changeset-bot
Copy link

changeset-bot bot commented Nov 5, 2025

⚠️ No Changeset found

Latest commit: df9120e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Nov 5, 2025

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

Project Deployment Preview Comments Updated (UTC)
dashboard Ready Ready Preview Comment Nov 5, 2025 9:22pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
engineering Ignored Ignored Preview Nov 5, 2025 9:22pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 5, 2025

📝 Walkthrough

Walkthrough

This pull request updates CI/CD workflows to add testing for the logdrain app, modifies path filters in change detection to exclude Markdown and TXT files, updates a comment in the dashboard page component, and reformats JSON arrays in the documentation manifest file.

Changes

Cohort / File(s) Summary
GitHub Actions Workflow Updates
.github/workflows/job_detect_changes.yaml, .github/workflows/job_test_logdrain.yaml, .github/workflows/pr.yaml
Enhanced path filters to exclude Markdown and TXT files from logdrain detection; added new "Test Logdrain" workflow job that runs TypeScript type checks and wrangler dry-run builds; integrated new job into pull request workflow with appropriate conditionals and dependencies
Dashboard Component
apps/dashboard/app/new/page.tsx
Updated comment text to capitalize "Ensure" and add "before proceeding" to first comment; capitalized "We" in second comment; no functional logic changes
Documentation Manifest
apps/docs/docs.json
Reformatted page arrays from multi-line to single-line format for identity concepts and error entries; content unchanged

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Workflow changes are structural/configuration updates without complex logic
  • Comment update is trivial text-only modification
  • JSON reformatting is purely cosmetic
  • Changes are localized to specific files with minimal interdependencies

Possibly related PRs

  • #3176: Modifies the same change-detection workflow for logdrain path filtering updates
  • #4210: Updates workflow files and adds component-specific test jobs with similar patterns
  • #4242: Also edits apps/docs/docs.json with documentation manifest formatting

Suggested labels

Core Team, Papercut

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description uses the standard template but is left incomplete with no specific issue reference, no type of change selected, no testing details provided, and all checklist items unchecked. Fill in the issue reference (Fixes #ENG-2151), select the appropriate change type, describe testing instructions, and check off completed checklist items before merging.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly indicates this is a test/placeholder PR that should not be merged, and references the related issue ENG-2151. It accurately describes the PR's purpose.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch test/dashboard-code-change

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a1f538 and df9120e.

📒 Files selected for processing (5)
  • .github/workflows/job_detect_changes.yaml (1 hunks)
  • .github/workflows/job_test_logdrain.yaml (1 hunks)
  • .github/workflows/pr.yaml (1 hunks)
  • apps/dashboard/app/new/page.tsx (1 hunks)
  • apps/docs/docs.json (2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: imeyer
Repo: unkeyed/unkey PR: 3755
File: .github/workflows/runbook-freshness-check.yaml:157-173
Timestamp: 2025-08-08T14:59:52.283Z
Learning: Repo unkeyed/unkey: When a CI/workflow fix is deferred, imeyer prefers a thorough GitHub issue be opened with sections (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References) and assigned to imeyer, including backlinks to the originating PR and comment.
📚 Learning: 2025-06-02T11:08:56.397Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3292
File: apps/dashboard/lib/vault.ts:80-97
Timestamp: 2025-06-02T11:08:56.397Z
Learning: The vault.ts file in apps/dashboard/lib/vault.ts is a duplicate of the vault package from the `api` directory and should be kept consistent with that original implementation.

Applied to files:

  • apps/dashboard/app/new/page.tsx
📚 Learning: 2024-10-25T23:53:41.716Z
Learnt from: Srayash
Repo: unkeyed/unkey PR: 2568
File: apps/dashboard/app/auth/sign-up/oauth-signup.tsx:25-25
Timestamp: 2024-10-25T23:53:41.716Z
Learning: In the React component `OAuthSignUp` (`apps/dashboard/app/auth/sign-up/oauth-signup.tsx`), adding a `useEffect` cleanup function to reset the `isLoading` state causes a "something went wrong" popup to appear before redirecting when a user clicks on signup.

Applied to files:

  • apps/dashboard/app/new/page.tsx
📚 Learning: 2025-08-08T19:52:42.113Z
Learnt from: imeyer
Repo: unkeyed/unkey PR: 3765
File: .github/workflows/job_detect_changes.yaml:43-43
Timestamp: 2025-08-08T19:52:42.113Z
Learning: In the unkeyed/unkey repository, the workflows `.github/workflows/job_test_api_local.yaml` and `.github/workflows/job_test_go_api_local.yaml` should keep their Blacksmith self-hosted runners (blacksmith-4vcpu-ubuntu-2404 and blacksmith-8vcpu-ubuntu-2404 respectively) as these are performance-critical test jobs that require dedicated resources.

Applied to files:

  • .github/workflows/pr.yaml
  • .github/workflows/job_test_logdrain.yaml
🪛 actionlint (1.7.8)
.github/workflows/job_test_logdrain.yaml

9-9: label "depot-ubuntu-24.04-4" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Test Dashboard / Test Dashboard
🔇 Additional comments (6)
.github/workflows/job_detect_changes.yaml (1)

103-104: Pattern consistency with dashboard filters — ✓

The logdrain path filters mirror the dashboard filters (lines 77–78) to exclude Markdown and TXT files. This is semantically sound: documentation-only changes should not trigger test runs.

apps/dashboard/app/new/page.tsx (1)

8-9: Comment improvements — ✓

Capitalization adjustments improve readability; no functional changes.

apps/docs/docs.json (1)

116-116: JSON array reformatting — ✓

Cosmetic reformatting of pages arrays to single-line format. No schema or data changes; documentation rendering is unaffected.

Also applies to: 272-272

.github/workflows/job_test_logdrain.yaml (2)

1-9: Workflow structure is sound — ✓

The logdrain test job follows the established pattern (checkout, Node/Wrangler setup, type check, dry-run build). The depot-ubuntu-24.04-4 runner is a custom Depot CI label, so the actionlint warning is a false positive.

Confirm that the depot-ubuntu-24.04-4 Depot runner label is configured in your repository settings or CI provider configuration. If not already documented, consider adding a comment to actionlint config or this workflow to suppress the warning.


20-29: Test coverage is appropriate — ✓

TypeScript type checking and a dry-run deploy test catch both static and build-time issues for Logdrain before actual deployment.

.github/workflows/pr.yaml (1)

43-47: New test job integration — ✓

The test_logdrain job follows the same conditional and dependency pattern as existing test jobs. It will run only when logdrain changes (excluding Markdown/TXT files) are detected.


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.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2025

Thank you for following the naming conventions for pull request titles! 🙏

@mcstepp mcstepp mentioned this pull request Nov 5, 2025
19 tasks
@mcstepp mcstepp force-pushed the test/dashboard-code-change branch from 3801011 to 7763fe6 Compare November 5, 2025 21:18
@vercel vercel bot temporarily deployed to Preview – dashboard November 5, 2025 21:22 Inactive
@mcstepp mcstepp closed this Nov 6, 2025
@mcstepp mcstepp deleted the test/dashboard-code-change branch November 6, 2025 15:53
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.

1 participant