Skip to content

fix(hiring): minor review fixes from #8369 (admin-access test, plan doc) - #8378

Merged
Kitenite merged 1 commit into
mainfrom
hiring-minor-review-fixes
Oct 11, 2026
Merged

Kitenite merged 1 commit into
mainfrom
hiring-minor-review-fixes

Conversation

@Kitenite

@Kitenite Kitenite commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

Two of the five minor review findings from #8369 still apply to this repo:

  • admin-access test: admin-access.test.ts builds its emails from COMPANY.EMAIL_DOMAIN, not a hardcoded @superset.sh. The old test fails with NEXT_PUBLIC_ROOT_DOMAIN=boid.so; the new one passes.
  • Plan doc: plans/admin-hiring-ats-design.md now lists follow-up in the bun hiring commands.

Not in this PR: #8374 moved the code for the other three findings to the private superset-sh/admin repo, so they must be fixed there:

  • EventTimeline: an empty authorLabel
  • schema.ts authorLabel
  • hiring-args parseFollowUp calendar validation
  • web-url toWebUrl
  • safeHref

Checks

  • bun run lint: passed
  • bun test in packages/trpc: 559 passed, 0 failed
  • bunx tsc --noEmit -p . in packages/trpc: the same 22 better-auth errors that are on main (auth, trpc, api-key, automation, organization, team), and no new ones
  • apps/admin no longer exists in this repo, so there was no tsc run for it

Summary by cubic

Fixes the last two review findings from #8369 that still apply to this repo.

  • admin-access.test.ts builds test emails from COMPANY.EMAIL_DOMAIN so the suite passes regardless of the configured root domain.
  • plans/admin-hiring-ats-design.md now lists follow-up in the bun hiring commands.

Written for commit d666b5d. Summary will update on new commits.

View guided diff Turn on auto-fix

…; list follow-up in plan

Remaining minor review fixes from #8369 that still apply to this repo.
The EventTimeline, hiring-args, web-url and safeHref fixes target code
that #8374 moved to superset-sh/admin.
@coderabbitai

coderabbitai Bot commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e097e3e2-3f85-4c88-9947-667187f0cb77

📥 Commits

Reviewing files that changed from the base of the PR and between a480d52 and d666b5d.


📒 Files selected for processing (2)
  • packages/trpc/src/lib/admin-access/admin-access.test.ts
  • plans/admin-hiring-ats-design.md

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch

Thank you for your contribution! 🎉

@greptile-apps

greptile-apps Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Low impact] This PR appears safe to merge.

Summary

Updates the admin-access test to use the configured company email domain and adds follow-up to the hiring plan’s command list.

  • Admin-access tests use the configured company email domain.
  • The hiring plan lists follow-up among terminal commands.

Reviews (1) · Last reviewed commit: "fix(hiring): build admin-access test ema..." · Reviewed by Greptile

@expo

expo Bot commented Oct 11, 2026

Copy link
Copy Markdown

Preview update published for development build 44d37656 (fingerprint 52085cf7de79). Open the build on your phone and scan:

Update group 171f40aa

@Kitenite
Kitenite merged commit 357c660 into main Oct 11, 2026
19 checks passed

This branch was previously deployed

1 inactive deployment
preview — d666b5dd Deployed Oct 11, 2026 by Kitenite via Deploy Web #11653
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