feat: delete dimensions and releases; rich release views - #385
Open
yuvrajjsingh0 wants to merge 6 commits into
Open
feat: delete dimensions and releases; rich release views#385yuvrajjsingh0 wants to merge 6 commits into
yuvrajjsingh0 wants to merge 6 commits into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe change adds custom and auto-generated release views, deletion releases, pending deletion tracking, dimension deletion authorization, and missing-dimension validation in release targeting. Dashboard pages and documentation expose the new states and workflows. ChangesRelease view deletion workflow
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Dashboard
participant DeleteSliceRelease
participant DeleteReleaseAPI
participant ReleaseViewStore
participant ReleaseExperiment
Dashboard->>DeleteSliceRelease: confirm view deletion
DeleteSliceRelease->>DeleteReleaseAPI: POST view ID
DeleteReleaseAPI->>ReleaseViewStore: validate view and pending state
DeleteReleaseAPI->>ReleaseExperiment: create deletion release
DeleteReleaseAPI->>ReleaseViewStore: mark pending deletion
ReleaseExperiment-->>Dashboard: display deletion release status
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
airborne_dashboard/components/release/steps/TargetingStep.tsx (1)
42-66: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winA failed dimension fetch flags every rule as deleted and blocks the wizard.
finallysetsdimensionsLoadedtotrueeven when the request fails.dimensionsthen stays[], so line 65 marks every rule dimension as unknown andcanProceedToStep(2)returnsfalse. Each card also shows "no longer exists", which is wrong. A transient network error therefore blocks release creation with a misleading message.Set the flag only after a successful load.
🐛 Proposed fix
setDimensions(data); + setDimensionsLoaded(true); } catch (err) { console.error("Error fetching dimensions:", err); - } finally { - setDimensionsLoaded(true); } };🤖 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 `@airborne_dashboard/components/release/steps/TargetingStep.tsx` around lines 42 - 66, Update the dimension-loading effect in TargetingStep so dimensionsLoaded is set to true only after apiFetch successfully returns and setDimensions completes; remove the unconditional finally update. Keep the existing unknown-dimension calculation gated by dimensionsLoaded, so failed requests do not mark all targeting rules as deleted or block the wizard.airborne_dashboard/app/dashboard/[orgId]/[appId]/views/page.tsx (1)
78-119: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDiscard stale list responses after the filter changes.
fetchViewsListcapturesfilterTypeand always writes toviewsList. If a user changes the filter twice quickly, the earlier request can resolve last and populate the list with views for the previous filter. The list then shows the wrong view types until the next refetch.Track the active request and ignore outdated responses.
🛠️ Proposed fix using a request token
+ const requestIdRef = useRef(0); + const fetchViewsList = async (pageNum: number = 1, append: boolean = false) => { + const requestId = ++requestIdRef.current; if (append) { setIsLoadingMore(true); } else { setIsLoading(true); } try { const res: ReleaseViewListResponse = await apiFetch( `/organisations/applications/dimension/release-view/list`, { query: { page: pageNum, count: 20, ...(filterType !== ViewTypeFilter.ALL ? { view_type: filterType } : {}), }, }, { token, org, app, } ); + if (requestId !== requestIdRef.current) return; + if (append) { setViewsList((prev) => [...prev, ...res.data]); } else { setViewsList(res.data); }🤖 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 `@airborne_dashboard/app/dashboard/`[orgId]/[appId]/views/page.tsx around lines 78 - 119, Update fetchViewsList to track a request token or equivalent active-request identifier for each filter-dependent fetch, and only apply viewsList, totalItems, and hasMore updates when the response belongs to the latest request. Ensure outdated responses are ignored, including their loading-state cleanup where necessary, while preserving append behavior for the active request.
🧹 Nitpick comments (5)
airborne_server/migrations/20260812120000_add_view_type_to_release_views/up.sql (1)
7-8: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAvoid validating this constraint while adding it.
ADD CONSTRAINTvalidates existing rows before it completes. This can block writes tohyperotaserver.release_viewsduring deployment. Add the constraint asNOT VALID, then validate it in a later migration.Proposed migration change
ALTER TABLE hyperotaserver.release_views ADD CONSTRAINT release_views_view_type_check - CHECK (view_type IN ('custom', 'auto_generated')); + CHECK (view_type IN ('custom', 'auto_generated')) NOT VALID;🤖 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 `@airborne_server/migrations/20260812120000_add_view_type_to_release_views/up.sql` around lines 7 - 8, Update the release_views_view_type_check constraint in the migration to add it as NOT VALID, avoiding validation of existing rows during deployment. Leave validation for a separate later migration.Source: Linters/SAST tools
airborne_dashboard/components/releaseViews/DeleteSliceRelease.tsx (1)
42-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEncode the path segment for consistency.
Other call sites in the dashboard wrap identifiers with
encodeURIComponent, for example the release detail requests.view.idis a server-generated UUID, so this is a consistency point rather than a defect.- `/releases/views/${view.id}/delete`, + `/releases/views/${encodeURIComponent(view.id)}/delete`,🤖 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 `@airborne_dashboard/components/releaseViews/DeleteSliceRelease.tsx` around lines 42 - 46, Update the API request in the DeleteSliceRelease component to wrap view.id with encodeURIComponent when constructing the /releases/views/{id}/delete path, matching the encoding used by other release-related requests while preserving the existing POST options and parameters.airborne_server/src/release.rs (1)
1208-1266: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider reconciling the created experiment if the pending marker fails to persist.
mark_delete_release_pendingat line 1257 runs after the experiment exists in Superposition. If that database write fails, the endpoint returns an error, but the delete experiment stays inCREATEDstate with no view marker. The view then still offers "Delete release", and a second call creates another experiment.Consider discarding the created experiment when the marker write fails, or logging the orphaned release ID at
errorlevel so it can be found.🤖 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 `@airborne_server/src/release.rs` around lines 1208 - 1266, Handle failure of release_view::mark_delete_release_pending after create_experiment succeeds by reconciling the already-created experiment: discard it when supported, or log the orphaned release_id at error level before propagating the database error. Keep the successful response unchanged and ensure the created experiment ID is available to the failure path.airborne_server/src/release/utils.rs (2)
527-557: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the new helpers inside
build_overrides.
build_overridesinlines the same resolve-config builder at lines 637-650 and the samebuild.filter at lines 935-947. The logic is now duplicated in two places, and it can drift. Callresolve_config_documentandconfig_document_to_overridesfrombuild_overrides, keeping the existing degrade-to-Nonebehavior where it is required.Also applies to: 611-626
🤖 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 `@airborne_server/src/release/utils.rs` around lines 527 - 557, Update build_overrides to call resolve_config_document for resolved configuration and config_document_to_overrides for converting the document, removing its duplicated builder and build-filter logic. Preserve build_overrides’ existing behavior of degrading helper failures to None, while keeping resolve_config_document’s error propagation for its direct callers.
536-549: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNon-string dimension values become empty strings in the resolve context.
Line 546 uses
value.as_str().unwrap_or(""). A numeric or boolean dimension value therefore resolves as"". Fordelete_release_for_viewthis makes the control snapshot resolve the wrong slice, so the "nothing to delete" comparison atrelease.rsline 1201 can compare the wrong configs.
build_overridesalready contains the same coercion, so this is pre-existing behavior. Consider serializing non-string values withvalue.to_string()for scalars in both places.🤖 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 `@airborne_server/src/release/utils.rs` around lines 536 - 549, The resolve context builder in the shown fold currently converts non-string dimension values to empty strings; update it to preserve scalar values by serializing them with the existing JSON value representation (for example, string content or value.to_string()). Apply the same conversion in build_overrides so both paths resolve dimensions consistently, while retaining empty handling only for genuinely unsupported or missing values.
🤖 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 `@airborne_dashboard/components/release/ReleaseFormContext.tsx`:
- Around line 213-215: Update the validation gate in TargetingStep so
unknownDimensions blocks progression only when targeting is editable; preserve
the current unknown-dimension check for create mode while allowing edit mode to
proceed even when a deleted dimension is present.
In `@airborne_server/src/organisation/application/dimension.rs`:
- Around line 405-431: Update the context construction and comparison in the
affected_views/release matching flow to preserve non-string JSON values instead
of converting them with as_str().unwrap_or_default(). Use direct JSON value
comparison or a lossless serialization consistently for both experiment and view
contexts, ensuring numeric and boolean dimensions remain distinct and release
matching cannot bind unrelated entries.
In `@airborne_server/src/release.rs`:
- Around line 1112-1142: Make the pending-deletion transition in
mark_delete_release_pending atomic with its check: update the view only when
pending_delete_release_id is NULL, inspect the affected-row count, and return
the existing “deletion already in progress” error when no row is updated. Keep
the earlier validation in the surrounding release-delete flow, but ensure only
the request that successfully claims the row can create the delete experiment.
- Around line 113-138: Update create_release to use the created experimental
variant ID (experimental_{pkg_version}) when concluding the first release
instead of constructing {experiment_id}-experimental_1. Pass
experimental_variant_id through the conclusion request, and propagate any
conclude error rather than discarding it so the endpoint cannot report success
while the release remains unconcluded.
In `@airborne_server/src/release/utils.rs`:
- Around line 572-586: Update the dimension lookup around list_dimensions to
fetch every page before building known. Set the request’s page/count parameters
and iterate through subsequent pages, aggregating each returned dimension into
the existing known set before filtering dims for missing entries.
In `@airborne_server/src/utils/release_view.rs`:
- Around line 190-205: The delete-release flow must reserve the view before
creating the external release to prevent concurrent overwrites. Update
mark_delete_release_pending and its caller to generate the release ID first,
atomically set pending_delete_col only when it is NULL, and treat a failed
update as reservation failure; if external release creation fails, clear only
the matching reservation before returning the error.
- Around line 150-170: Update the release-view creation flow to keep custom and
auto-generated views independent: in the logic around existing and the diesel
insert, avoid treating matching custom dimensions as an existing auto-generated
view, and ensure the uniqueness/conflict contract distinguishes view_type (or
otherwise uses a separate auto-generated name namespace). Preserve
delete_release_for_view compatibility by allowing auto-generated views to be
inserted and conflict only with equivalent auto-generated records.
---
Outside diff comments:
In `@airborne_dashboard/app/dashboard/`[orgId]/[appId]/views/page.tsx:
- Around line 78-119: Update fetchViewsList to track a request token or
equivalent active-request identifier for each filter-dependent fetch, and only
apply viewsList, totalItems, and hasMore updates when the response belongs to
the latest request. Ensure outdated responses are ignored, including their
loading-state cleanup where necessary, while preserving append behavior for the
active request.
In `@airborne_dashboard/components/release/steps/TargetingStep.tsx`:
- Around line 42-66: Update the dimension-loading effect in TargetingStep so
dimensionsLoaded is set to true only after apiFetch successfully returns and
setDimensions completes; remove the unconditional finally update. Keep the
existing unknown-dimension calculation gated by dimensionsLoaded, so failed
requests do not mark all targeting rules as deleted or block the wizard.
---
Nitpick comments:
In `@airborne_dashboard/components/releaseViews/DeleteSliceRelease.tsx`:
- Around line 42-46: Update the API request in the DeleteSliceRelease component
to wrap view.id with encodeURIComponent when constructing the
/releases/views/{id}/delete path, matching the encoding used by other
release-related requests while preserving the existing POST options and
parameters.
In
`@airborne_server/migrations/20260812120000_add_view_type_to_release_views/up.sql`:
- Around line 7-8: Update the release_views_view_type_check constraint in the
migration to add it as NOT VALID, avoiding validation of existing rows during
deployment. Leave validation for a separate later migration.
In `@airborne_server/src/release.rs`:
- Around line 1208-1266: Handle failure of
release_view::mark_delete_release_pending after create_experiment succeeds by
reconciling the already-created experiment: discard it when supported, or log
the orphaned release_id at error level before propagating the database error.
Keep the successful response unchanged and ensure the created experiment ID is
available to the failure path.
In `@airborne_server/src/release/utils.rs`:
- Around line 527-557: Update build_overrides to call resolve_config_document
for resolved configuration and config_document_to_overrides for converting the
document, removing its duplicated builder and build-filter logic. Preserve
build_overrides’ existing behavior of degrading helper failures to None, while
keeping resolve_config_document’s error propagation for its direct callers.
- Around line 536-549: The resolve context builder in the shown fold currently
converts non-string dimension values to empty strings; update it to preserve
scalar values by serializing them with the existing JSON value representation
(for example, string content or value.to_string()). Apply the same conversion in
build_overrides so both paths resolve dimensions consistently, while retaining
empty handling only for genuinely unsupported or missing values.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fcca68e5-5807-450d-b769-31758977bf9d
⛔ Files ignored due to path filters (10)
airborne_docs/static/docs_static/img/screenshots/dark/dimension-delete-blocked.pngis excluded by!**/*.pngairborne_docs/static/docs_static/img/screenshots/dark/dimensions-list.pngis excluded by!**/*.pngairborne_docs/static/docs_static/img/screenshots/dark/releases-list.pngis excluded by!**/*.pngairborne_docs/static/docs_static/img/screenshots/dark/views-delete-release.pngis excluded by!**/*.pngairborne_docs/static/docs_static/img/screenshots/dark/views.pngis excluded by!**/*.pngairborne_docs/static/docs_static/img/screenshots/light/dimension-delete-blocked.pngis excluded by!**/*.pngairborne_docs/static/docs_static/img/screenshots/light/dimensions-list.pngis excluded by!**/*.pngairborne_docs/static/docs_static/img/screenshots/light/releases-list.pngis excluded by!**/*.pngairborne_docs/static/docs_static/img/screenshots/light/views-delete-release.pngis excluded by!**/*.pngairborne_docs/static/docs_static/img/screenshots/light/views.pngis excluded by!**/*.png
📒 Files selected for processing (25)
airborne_dashboard/app/dashboard/[orgId]/[appId]/dimensions/page.tsxairborne_dashboard/app/dashboard/[orgId]/[appId]/releases/[releaseId]/page.tsxairborne_dashboard/app/dashboard/[orgId]/[appId]/releases/page.tsxairborne_dashboard/app/dashboard/[orgId]/[appId]/views/page.tsxairborne_dashboard/components/dimensions/DeleteDimension.tsxairborne_dashboard/components/release/ReleaseFormContext.tsxairborne_dashboard/components/release/steps/TargetingStep.tsxairborne_dashboard/components/releaseViews/DeleteSliceRelease.tsxairborne_dashboard/types/release.tsairborne_docs/docs/dashboard/dimensions.mdxairborne_docs/docs/dashboard/releases.mdxairborne_docs/docs/dashboard/views.mdxairborne_server/migrations/20260812120000_add_view_type_to_release_views/down.sqlairborne_server/migrations/20260812120000_add_view_type_to_release_views/up.sqlairborne_server/migrations/20260812130000_add_pending_delete_to_release_views/down.sqlairborne_server/migrations/20260812130000_add_pending_delete_to_release_views/up.sqlairborne_server/src/organisation/application/dimension.rsairborne_server/src/organisation/application/dimension/types.rsairborne_server/src/release.rsairborne_server/src/release/types.rsairborne_server/src/release/utils.rsairborne_server/src/utils.rsairborne_server/src/utils/db/models.rsairborne_server/src/utils/db/schema.rsairborne_server/src/utils/release_view.rs
yuvrajjsingh0
force-pushed
the
feat/delete-dimensions-releases-rich-release-views
branch
2 times, most recently
from
August 17, 2026 13:29
4dfc39a to
566b51c
Compare
yuvrajjsingh0
force-pushed
the
feat/delete-dimensions-releases-rich-release-views
branch
from
August 18, 2026 07:40
566b51c to
9bd6bd7
Compare
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
1 participant
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
Documentation