feat: suggest bounded expirations for policy skips in ui#1170
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR refactors the deployment skip expiry workflow by extracting shared logic into utilities and components. A new ChangesDeployment Skip Expiry Selection Flow
Sequence DiagramsequenceDiagram
participant User
participant CreateSkipForm
participant Evaluations API
participant Skip Mutation
User->>CreateSkipForm: Select rule
CreateSkipForm->>Evaluations API: Query deploymentVersions.evaluate
Evaluations API-->>CreateSkipForm: Return rule evaluations
CreateSkipForm->>CreateSkipForm: expiryOptionsForRule(evaluations)
CreateSkipForm->>User: Render ExpirySelect with computed options
User->>CreateSkipForm: Select expiry option
User->>CreateSkipForm: Click "Add skip"
CreateSkipForm->>Skip Mutation: policySkips.createForEnvAndVersion
Skip Mutation-->>CreateSkipForm: Success
CreateSkipForm->>CreateSkipForm: Reset state, invalidate cache
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the deployments UI skip flows to encourage bounded (time-limited) policy skips by replacing free-form datetime selection with a small set of suggested expirations, including rule-derived “until X completes/opens/ends/passes” options when available.
Changes:
- Added shared logic to derive skip expiration options from policy evaluation details (plus fixed-duration fallbacks).
- Introduced a reusable
ExpirySelectUI component to present those options and warn about permanent skips. - Updated both release-target skip actions and env+version skip creation to use the new expiry selector instead of a datetime picker.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
apps/web/app/routes/ws/deployments/_components/release-targets/skip-expiry.ts |
New option-building utilities to suggest bounded expirations based on evaluation details. |
apps/web/app/routes/ws/deployments/_components/release-targets/PolicySkipActions.tsx |
Replaces datetime picker with suggested expiry selector in release-target skip popover. |
apps/web/app/routes/ws/deployments/_components/release-targets/ExpirySelect.tsx |
New select component for expiry choices + UX warning for “No expiration”. |
apps/web/app/routes/ws/deployments/_components/environmentversiondecisions/policy-skip/CreateSkipForm.tsx |
Refactors env+version skip creation to use suggested expiry selector and local state. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+100
to
+115
| export function expiryOptionsForRule( | ||
| evaluations: RuleEvaluation[], | ||
| ruleId: string | undefined, | ||
| now: Date, | ||
| ): ExpiryOption[] { | ||
| const ruleEvaluations = evaluations.filter((e) => e.ruleId === ruleId); | ||
| const first = ruleEvaluations[0]; | ||
| return buildExpiryOptions({ | ||
| details: first.details ?? null, | ||
| nextEvaluationAt: | ||
| first.nextEvaluationAt != null ? new Date(first.nextEvaluationAt) : null, | ||
| gradualMaxTime: | ||
| ruleId != null ? maxRolloutTime(ruleEvaluations, ruleId) : null, | ||
| now, | ||
| }); | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/web/app/routes/ws/deployments/_components/release-targets/skip-expiry.ts`:
- Around line 100-115: expiryOptionsForRule can dereference an undefined first
when no evaluations match ruleId; update expiryOptionsForRule to safely handle
empty ruleEvaluations by using optional chaining or guards before accessing
first.details and first.nextEvaluationAt and by passing nulls to
buildExpiryOptions when first is undefined; keep the call to
maxRolloutTime(ruleEvaluations, ruleId) only when ruleEvaluations is non-empty
(or ensure maxRolloutTime tolerates empty arrays) so that expiryOptionsForRule
(the function) never reads properties off an undefined first and
buildExpiryOptions always receives valid nullable fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f6c3c89f-ccb6-4ad3-a4f4-bb61c4a0c52c
📒 Files selected for processing (4)
apps/web/app/routes/ws/deployments/_components/environmentversiondecisions/policy-skip/CreateSkipForm.tsxapps/web/app/routes/ws/deployments/_components/release-targets/ExpirySelect.tsxapps/web/app/routes/ws/deployments/_components/release-targets/PolicySkipActions.tsxapps/web/app/routes/ws/deployments/_components/release-targets/skip-expiry.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/app/routes/ws/deployments/_components/release-targets/skip-expiry.ts (1)
54-56: 💤 Low valueTypo in comment perpetuates API naming issue.
The comment references
evaulatewhich matches the existing API endpointdeploymentVersions.evaulate. While technically accurate, this perpetuates a typo that may confuse future readers. Consider adding a clarifying note that this matches an existing API name, or filing a follow-up to rename the API endpoint.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/routes/ws/deployments/_components/release-targets/skip-expiry.ts` around lines 54 - 56, The comment contains a typo "evaulate" which mirrors the existing API endpoint deploymentVersions.evaulate; update the comment to either correct the spelling to "evaluate" while explicitly noting it matches the current API name (e.g., "note: API is named deploymentVersions.evaulate with this spelling") or add a brief clarifying sentence suggesting a follow-up task to rename the API; reference the symbol deploymentVersions.evaulate so readers understand the mismatch and avoid confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@apps/web/app/routes/ws/deployments/_components/release-targets/skip-expiry.ts`:
- Around line 54-56: The comment contains a typo "evaulate" which mirrors the
existing API endpoint deploymentVersions.evaulate; update the comment to either
correct the spelling to "evaluate" while explicitly noting it matches the
current API name (e.g., "note: API is named deploymentVersions.evaulate with
this spelling") or add a brief clarifying sentence suggesting a follow-up task
to rename the API; reference the symbol deploymentVersions.evaulate so readers
understand the mismatch and avoid confusion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eead1526-df6f-4d20-8aa5-57670cd66062
📒 Files selected for processing (1)
apps/web/app/routes/ws/deployments/_components/release-targets/skip-expiry.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
UI/UX Improvements