-
-
Notifications
You must be signed in to change notification settings - Fork 789
feat: show gh username and avatar for deployments triggered by the github app integration #2394
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
Conversation
|
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (23)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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
🧹 Nitpick comments (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (1)
321-328
: Minor UX: truncate long names and expose full value via titlePrevents layout shifts in narrow columns and preserves the full name via tooltip.
function UserTag({ name, avatarUrl }: { name: string; avatarUrl?: string }) { return ( <div className="flex items-center gap-1"> <UserAvatar avatarUrl={avatarUrl} name={name} className="h-4 w-4" /> - <Paragraph variant="extra-small">{name}</Paragraph> + <Paragraph variant="extra-small" title={name} className="truncate max-w-40"> + {name} + </Paragraph> </div> ); }apps/webapp/app/presenters/v3/BranchesPresenter.server.ts (1)
42-47
: Couple ‘source’ to the core schema to avoid drift; clarify docs for new fieldsUse the type from the GitMeta zod schema to future-proof the union, and document the semantics of the new fields.
/** The commit author */ commitAuthor: string; /** The git provider, e.g., `github` */ provider?: string; - source?: "trigger_github_app" | "github_actions" | "local"; - ghUsername?: string; - ghUserAvatarUrl?: string; + /** The deployment source, as defined in the core GitMeta schema */ + source?: z.infer<typeof GitMeta>["source"]; + /** Username of the actor when triggered via the GitHub App (if available) */ + ghUsername?: string; + /** Avatar URL of the actor when triggered via the GitHub App (if available) */ + ghUserAvatarUrl?: string;Note: Making these optional matches the established pattern for partial Git metadata (per prior learnings).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/webapp/app/presenters/v3/BranchesPresenter.server.ts
(3 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}
: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
apps/webapp/app/presenters/v3/BranchesPresenter.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
apps/webapp/app/presenters/v3/BranchesPresenter.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}
: In the webapp, all environment variables must be accessed through theenv
export ofenv.server.ts
, instead of directly accessingprocess.env
.
When importing from@trigger.dev/core
in the webapp, never import from the root@trigger.dev/core
path; always use one of the subpath exports as defined in the package's package.json.
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
apps/webapp/app/presenters/v3/BranchesPresenter.server.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2392
File: packages/cli-v3/src/utilities/gitMeta.ts:195-218
Timestamp: 2025-08-14T12:13:20.408Z
Learning: In the GitMeta schema (packages/core/src/v3/schemas/common.ts), all fields are intentionally optional to handle partial data from various deployment contexts (local, GitHub Actions, GitHub App). Functions like getGitHubAppMeta() are designed to work with missing environment variables rather than validate their presence.
📚 Learning: 2025-08-14T12:13:20.408Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2392
File: packages/cli-v3/src/utilities/gitMeta.ts:195-218
Timestamp: 2025-08-14T12:13:20.408Z
Learning: In the GitMeta schema (packages/core/src/v3/schemas/common.ts), all fields are intentionally optional to handle partial data from various deployment contexts (local, GitHub Actions, GitHub App). Functions like getGitHubAppMeta() are designed to work with missing environment variables rather than validate their presence.
Applied to files:
apps/webapp/app/presenters/v3/BranchesPresenter.server.ts
⏰ 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). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/webapp/app/presenters/v3/BranchesPresenter.server.ts (2)
3-3
: Type-only import is spot onSwitching Prisma/PrismaClient to type-only while keeping the prisma value reduces runtime footprint and aligns with TS best practices.
249-253
: Wiring through the new metadata fields looks correctMapping provider, source, ghUsername, and ghUserAvatarUrl from the parsed GitMeta is consistent with the schema and preserves optionality.
If any of these properties are not yet present in the GitMeta schema in @trigger.dev/core/v3, they’ll always be undefined at runtime. Please confirm the core schema exports these fields. If not, I can open a follow-up PR to add them.
...outes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
Outdated
Show resolved
Hide resolved
…hub app integration Small change that shows the github user in the deployments page in cases where it was triggered by the github app integration. These deployments are not triggered using a PAT, so we cannot associate them with a Trigger user ID.
fae2f75
to
fd77372
Compare
Small change that shows the github user in the deployments page in cases
where it was triggered by the github app integration.
These deployments are not triggered using a PAT, so we cannot associate
them with a Trigger user ID.