feat: add mass backup button on project page#254
feat: add mass backup button on project page#254romzes5000 wants to merge 9 commits intoPortabase:mainfrom
Conversation
Add an optional pageSizeOptions prop to CardsWithPagination that renders a "Cards per page" dropdown selector, consistent with the existing TablePaginationSize pattern used in DataTable. - New PaginationSize component (standalone, no TanStack Table dependency) - CardsWithPagination: manage pageSize as state, reset to page 1 on change - Project databases page: increase default from 6 to 20, add selector [10, 20, 50] - Projects list page: increase default from 9 to 12, add selector [12, 24, 48] - Fully backward compatible: without pageSizeOptions, behavior is unchanged Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Validate pageSize in handlePageSizeChange (reject 0/negative/NaN) - Guard empty pageSizeOptions array from rendering empty selector - Restore className on PaginationNavigation for backward compatibility - Add aria-label to SelectTrigger for accessibility on small screens Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…t in options Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add a "Backup" button to the project detail page header that queues backup jobs for all databases in the project at once. - Server action with org membership + role check (not "member") - Skips databases that already have a backup waiting/ongoing - Check + insert inside a single transaction to prevent race conditions - Unified error messages to prevent project enumeration - No internal error details leaked to the client Made-with: Cursor
📝 WalkthroughWalkthroughAdds a mass-backup feature (client button + server action) to enqueue backups for project databases; introduces selectable page sizes in pagination, updates default cards-per-page values for projects and project databases, and adds a GitLab CI job to build/push a production Docker image. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as MassBackupButton
participant Mutation as ReactQueryMutation
participant Server as massBackupProjectAction
participant DB as Database
participant Toast as ToastService
participant Router as Router
User->>UI: Click "Mass backup"
UI->>User: Show confirmation dialog
User->>UI: Confirm
UI->>Mutation: mutate(projectId)
Mutation->>Server: call massBackupProjectAction(projectId)
Server->>Server: validate project, membership, role
Server->>DB: fetch project databases and existing backups
Server->>DB: insert backup rows in transaction for eligible DBs
DB-->>Server: return created/skipped counts
Server-->>Mutation: return success or failure payload
Mutation->>Toast: show success or error
Mutation->>Router: router.refresh() / invalidate queries
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`(customer)/dashboard/(organization)/projects/[projectId]/page.tsx:
- Around line 85-89: The MassBackupButton is being passed disabled={isMember}
while it is only rendered inside the !isMember branch, so disabled will always
be false; remove the redundant disabled prop from the JSX (the MassBackupButton
usage with projectId and databaseCount) or replace it with a meaningful
condition if there are other disable scenarios, and then update the
MassBackupButtonProps/type (e.g., MassBackupButtonProps) to make disabled
optional or remove the prop entirely so the component signature matches the
updated usage.
In
`@src/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-button.tsx`:
- Around line 23-41: The mutation currently performs side effects inside
mutationFn; change mutationFn (used in useMutation) to only call
massBackupProjectAction(projectId) and return its result (do not call toast,
queryClient.invalidateQueries, or router.refresh there). Add onSuccess callback
to inspect result.data.success and run toast.success,
queryClient.invalidateQueries({ queryKey: ["database-data"] }), and
router.refresh when appropriate, and add onError (or onSuccess branch for
result.data.actionError) to call toast.error with
result?.data?.actionError?.message || result?.serverError || "Failed to create
backup."; ensure useMutation references the same mutation variable and that any
non-HTTP errors thrown by massBackupProjectAction are propagated to onError.
- Around line 70-72: The confirm.onClick currently receives an async function
wrapper (onClick: async () => { await handleAction(); }) which returns a Promise
and can produce unhandled rejections; change it to a synchronous fire-and-forget
call using the mutation API (e.g., replace the async wrapper with () =>
mutation.mutate(...) and remove the handleAction wrapper), or alternatively wrap
the async call in a try/catch invoked inside a non-async function; update
references to handleAction and ensure you call mutation.mutate (not
mutation.mutateAsync) so ButtonWithConfirm.confirm.onClick is a () => void.
In
`@src/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-project.action.ts`:
- Around line 91-96: The loop is performing sequential inserts which is slow;
replace the per-item await loop over toBackup with a single batch insert by
constructing an array of objects like { databaseId: d.id, status: "waiting" }
from toBackup and passing that array to
tx.insert(drizzleDb.schemas.backup).values(...); ensure you still await the
single tx.insert(...) call (and include any necessary returning or timestamp
fields if required by backup schema) so all rows are inserted in one operation
instead of many awaits in the loop.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 49f1d8f5-ce26-448b-9a10-72f6a6dc8171
📒 Files selected for processing (3)
app/(customer)/dashboard/(organization)/projects/[projectId]/page.tsxsrc/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-button.tsxsrc/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-project.action.ts
📜 Review details
🔇 Additional comments (3)
src/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-project.action.ts (2)
40-58: LGTM: Authorization logic is well-implemented.The membership check correctly verifies both organization membership and role permissions, returning a unified error message to prevent project enumeration attacks.
76-102: Duplicate backups are already prevented by database constraint.A unique index
(database_id, status)already exists on the backups table in migration 0000_awesome_nomad.sql, preventing multiple backups with the same status per database. This makes the race condition concern moot—any concurrent insert of a duplicatewaitingbackup will fail at the constraint level rather than succeeding.However, constraint violations are caught by the generic error handler and return a 500 error without distinguishing the specific cause. Consider distinguishing constraint violations to provide a clearer error message (e.g., "Backup already in progress for this database") rather than a generic failure.
src/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-button.tsx (1)
47-82: LGTM: Button configuration and disabled state logic.The disabled state correctly combines all relevant conditions, and the
ButtonWithConfirmintegration properly handles the confirmation flow with appropriate UX messaging for the zero-database case.
src/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-button.tsx
Show resolved
Hide resolved
src/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-button.tsx
Outdated
Show resolved
Hide resolved
src/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-project.action.ts
Outdated
Show resolved
Hide resolved
- Remove redundant `disabled` prop (always false inside `!isMember` block) - Move side effects from mutationFn to onSuccess/onError callbacks - Replace async onClick with synchronous mutation.mutate - Use batch insert instead of sequential loop in transaction - Remove unused `sql` import Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-project.action.ts`:
- Around line 50-58: Replace the negative membership.role check with a positive
allowlist: instead of using membership.role === "member", define an explicit
set/array of permitted roles (e.g., allowedRoles including "owner" and "admin")
and verify membership.role is in that set before allowing the action; update the
conditional that currently returns the error object (the block that references
membership, membership.role, and parsedInput) to return the same error when
membership is missing or membership.role is not in the allowedRoles set.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 84162bc2-4fe7-4fa9-b1ba-5d98abe2d1c3
📒 Files selected for processing (3)
app/(customer)/dashboard/(organization)/projects/[projectId]/page.tsxsrc/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-button.tsxsrc/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-project.action.ts
📜 Review details
🔇 Additional comments (10)
src/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-project.action.ts (6)
1-14: LGTM!Clean imports and well-defined result type with appropriate
countandskippedfields for the mass backup operation.
22-38: LGTM!The project lookup correctly filters for non-archived projects, and the generic error message prevents project ID enumeration.
60-72: LGTM!Database fetch and empty project validation are implemented correctly.
76-102: LGTM!The transaction properly isolates the check-then-insert operation to prevent TOCTOU races. The batch insert pattern (addressing the previous review comment) is correctly implemented with proper type assertion for the status literal.
104-129: LGTM!Good differentiation between "all databases skipped" (returns failure) and partial success. The conditional success message appropriately informs users about skipped databases.
130-141: LGTM!Proper error handling: errors are logged server-side for debugging while returning a generic message to the client to prevent information leakage.
app/(customer)/dashboard/(organization)/projects/[projectId]/page.tsx (1)
85-88: LGTM!Clean integration of the
MassBackupButton. The redundantdisabledprop has been removed (addressing the previous review comment), and the component correctly receivesprojectIdanddatabaseCountfrom the project data.src/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-button.tsx (3)
1-15: LGTM!Clean imports and well-defined props type for the component.
22-43: LGTM!The mutation structure correctly follows React Query patterns with side effects in
onSuccess/onErrorcallbacks (addressing the previous review feedback). The query invalidation using["database-data"]as a prefix key will correctly invalidate all database data queries across the project due to React Query v5's default partial matching behavior.
45-80: LGTM!The component correctly disables when there are no databases or during pending state. The confirmation dialog provides clear context, with appropriate messaging for the empty-database edge case. The
onClickhandler properly uses synchronousmutation.mutate(addressing the previous review feedback).
| if (!membership || membership.role === "member") { | ||
| return { | ||
| success: false, | ||
| actionError: { | ||
| message: "Project not found or access denied.", | ||
| messageParams: { projectId: parsedInput }, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using an allowlist for role validation.
The current check membership.role === "member" denies only the member role, implicitly allowing any other value. Since the role column is an unconstrained text field (per schema), an unexpected value (typo, future role) would pass this check.
A positive allowlist is more defensive:
- if (!membership || membership.role === "member") {
+ if (!membership || !["admin", "owner"].includes(membership.role)) {This ensures only explicitly permitted roles can perform mass backups.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!membership || membership.role === "member") { | |
| return { | |
| success: false, | |
| actionError: { | |
| message: "Project not found or access denied.", | |
| messageParams: { projectId: parsedInput }, | |
| }, | |
| }; | |
| } | |
| if (!membership || !["admin", "owner"].includes(membership.role)) { | |
| return { | |
| success: false, | |
| actionError: { | |
| message: "Project not found or access denied.", | |
| messageParams: { projectId: parsedInput }, | |
| }, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-project.action.ts`
around lines 50 - 58, Replace the negative membership.role check with a positive
allowlist: instead of using membership.role === "member", define an explicit
set/array of permitted roles (e.g., allowedRoles including "owner" and "admin")
and verify membership.role is in that set before allowing the action; update the
conditional that currently returns the error object (the block that references
membership, membership.role, and parsedInput) to return the same error when
membership is missing or membership.role is not in the allowedRoles set.
Build linux/amd64 prod target; push ref slug, short SHA, and latest on default branch. Skip duplicate MR pipelines. Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`(customer)/dashboard/(organization)/projects/[projectId]/page.tsx:
- Around line 110-112: The default pagination props are inconsistent: update the
props passed to the page component so cardsPerPage and pageSizeOptions align
(either set cardsPerPage to the first entry of pageSizeOptions or reorder
pageSizeOptions so its first value equals cardsPerPage); specifically adjust the
cardsPerPage prop or the pageSizeOptions array used with the PaginationSize
component (cardsPerPage={20} and pageSizeOptions={[10,20,50]}) so the default
value appears as the first option for consistent behavior with the other pages
(e.g., make cardsPerPage match pageSizeOptions[0] or swap the array order).
In `@src/components/wrappers/common/cards-with-pagination.tsx`:
- Around line 26-28: The pageSize state in useState(cardsPerPage) (pageSize,
setPageSize) is only set on mount and won't update when the cardsPerPage prop
changes; add a sync useEffect that watches cardsPerPage and calls
setPageSize(cardsPerPage) (and optionally resets currentPage via
setCurrentPage(1)) so the component reflects prop changes; locate the useState
declarations for pageSize/setPageSize and currentPage/setCurrentPage in
cards-with-pagination.tsx and add the effect there.
In `@src/components/wrappers/common/pagination/pagination-size.tsx`:
- Around line 11-14: PaginationSize computes effectivePageSize by accessing
pageSizeOptions[0], which breaks if pageSizeOptions is an empty array; update
the logic in the PaginationSize component to guard against an empty
pageSizeOptions (e.g., treat an empty array as missing) by using a fallback
default array or fallback value: choose pageSizeOptions if it has length and
includes(props.pageSize) ? props.pageSize : pageSizeOptions[0], otherwise fall
back to props.pageSize if valid or a hardcoded default (e.g., 10); ensure
references to pageSizeOptions, props.pageSize, and effectivePageSize are
adjusted so effectivePageSize is never undefined before rendering the Select and
before calling onPageSizeChange.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8ddf8c0b-45a1-4918-abdd-c60468d3e2e0
📒 Files selected for processing (4)
app/(customer)/dashboard/(organization)/projects/[projectId]/page.tsxapp/(customer)/dashboard/(organization)/projects/page.tsxsrc/components/wrappers/common/cards-with-pagination.tsxsrc/components/wrappers/common/pagination/pagination-size.tsx
📜 Review details
🔇 Additional comments (5)
app/(customer)/dashboard/(organization)/projects/page.tsx (1)
69-71: LGTM!The pagination configuration is consistent -
cardsPerPage={12}correctly matches the first option inpageSizeOptions={[12, 24, 48]}, ensuring proper initialization of the page size selector.app/(customer)/dashboard/(organization)/projects/[projectId]/page.tsx (1)
85-88: LGTM!The
MassBackupButtonintegration is correct - props match the expected interface (projectId: string,databaseCount: number), and the component is properly placed within the!isMemberblock for role-based visibility. The server action provides the actual authorization enforcement.src/components/wrappers/common/pagination/pagination-size.tsx (1)
15-34: LGTM on accessibility and UI structure.Good implementation with proper
aria-labelon theSelectTriggerfor screen readers, and responsive design hiding the label on smaller screens while maintaining functionality.src/components/wrappers/common/cards-with-pagination.tsx (2)
46-50: LGTM on validation logic.The
handlePageSizeChangeproperly validates the input (Number.isFiniteand>= 1) and resets to page 1 to avoid showing an empty page after reducing page size.
61-78: LGTM on layout structure.The flex wrapper cleanly groups the optional
PaginationSizeandPaginationNavigationcontrols with consistent spacing. The conditional rendering viashowSizeSelectorcorrectly gates the size selector.
| cardsPerPage={20} | ||
| numberOfColumns={3} | ||
| pageSizeOptions={[10, 20, 50]} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent pagination defaults.
cardsPerPage={20} doesn't match the first option in pageSizeOptions={[10, 20, 50]}. The PaginationSize component falls back to pageSizeOptions[0] (which is 10) when the pageSize isn't in the options array. However, since 20 is in the array, this will work correctly.
That said, this pattern is inconsistent with projects/page.tsx where cardsPerPage={12} matches the first option [12, 24, 48]. Consider aligning for consistency:
♻️ Suggested alignment
Either change the default to match the first option:
- cardsPerPage={20}
+ cardsPerPage={10}Or reorder options so the default is first:
- pageSizeOptions={[10, 20, 50]}
+ pageSizeOptions={[20, 10, 50]}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cardsPerPage={20} | |
| numberOfColumns={3} | |
| pageSizeOptions={[10, 20, 50]} | |
| cardsPerPage={10} | |
| numberOfColumns={3} | |
| pageSizeOptions={[10, 20, 50]} |
| cardsPerPage={20} | |
| numberOfColumns={3} | |
| pageSizeOptions={[10, 20, 50]} | |
| cardsPerPage={20} | |
| numberOfColumns={3} | |
| pageSizeOptions={[20, 10, 50]} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`(customer)/dashboard/(organization)/projects/[projectId]/page.tsx around
lines 110 - 112, The default pagination props are inconsistent: update the props
passed to the page component so cardsPerPage and pageSizeOptions align (either
set cardsPerPage to the first entry of pageSizeOptions or reorder
pageSizeOptions so its first value equals cardsPerPage); specifically adjust the
cardsPerPage prop or the pageSizeOptions array used with the PaginationSize
component (cardsPerPage={20} and pageSizeOptions={[10,20,50]}) so the default
value appears as the first option for consistent behavior with the other pages
(e.g., make cardsPerPage match pageSizeOptions[0] or swap the array order).
| const [pageSize, setPageSize] = useState(cardsPerPage); | ||
| const [currentPage, setCurrentPage] = useState(1); | ||
| const totalPages = Math.ceil(data.length / cardsPerPage); | ||
| const totalPages = Math.ceil(data.length / pageSize); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
pageSize state won't sync if cardsPerPage prop changes.
The pageSize state is initialized from cardsPerPage only on first render. If the parent re-renders with a different cardsPerPage value, the component will continue using the stale initial value.
This may be intentional (user selection persists), but if the prop change should reset the state, consider adding a sync effect:
♻️ Optional: Sync state with prop changes
+import React, { ComponentType, useState, useEffect } from "react";
// Inside component:
const [pageSize, setPageSize] = useState(cardsPerPage);
+
+useEffect(() => {
+ setPageSize(cardsPerPage);
+ setCurrentPage(1);
+}, [cardsPerPage]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/wrappers/common/cards-with-pagination.tsx` around lines 26 -
28, The pageSize state in useState(cardsPerPage) (pageSize, setPageSize) is only
set on mount and won't update when the cardsPerPage prop changes; add a sync
useEffect that watches cardsPerPage and calls setPageSize(cardsPerPage) (and
optionally resets currentPage via setCurrentPage(1)) so the component reflects
prop changes; locate the useState declarations for pageSize/setPageSize and
currentPage/setCurrentPage in cards-with-pagination.tsx and add the effect
there.
| export const PaginationSize = (props: PaginationSizeProps) => { | ||
| const { className, onPageSizeChange, pageSizeOptions = [10, 20, 30, 40, 50] } = props; | ||
| const effectivePageSize = pageSizeOptions.includes(props.pageSize) ? props.pageSize : pageSizeOptions[0]; | ||
|
|
There was a problem hiding this comment.
Guard against empty pageSizeOptions array.
If pageSizeOptions is passed as an empty array [], pageSizeOptions[0] will be undefined, causing effectivePageSize to be undefined and rendering issues in the Select component.
🛡️ Proposed defensive check
export const PaginationSize = (props: PaginationSizeProps) => {
- const { className, onPageSizeChange, pageSizeOptions = [10, 20, 30, 40, 50] } = props;
+ const { className, onPageSizeChange, pageSizeOptions: rawOptions = [10, 20, 30, 40, 50] } = props;
+ const pageSizeOptions = rawOptions.length > 0 ? rawOptions : [10, 20, 30, 40, 50];
const effectivePageSize = pageSizeOptions.includes(props.pageSize) ? props.pageSize : pageSizeOptions[0];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const PaginationSize = (props: PaginationSizeProps) => { | |
| const { className, onPageSizeChange, pageSizeOptions = [10, 20, 30, 40, 50] } = props; | |
| const effectivePageSize = pageSizeOptions.includes(props.pageSize) ? props.pageSize : pageSizeOptions[0]; | |
| export const PaginationSize = (props: PaginationSizeProps) => { | |
| const { className, onPageSizeChange, pageSizeOptions: rawOptions = [10, 20, 30, 40, 50] } = props; | |
| const pageSizeOptions = rawOptions.length > 0 ? rawOptions : [10, 20, 30, 40, 50]; | |
| const effectivePageSize = pageSizeOptions.includes(props.pageSize) ? props.pageSize : pageSizeOptions[0]; | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/wrappers/common/pagination/pagination-size.tsx` around lines
11 - 14, PaginationSize computes effectivePageSize by accessing
pageSizeOptions[0], which breaks if pageSizeOptions is an empty array; update
the logic in the PaginationSize component to guard against an empty
pageSizeOptions (e.g., treat an empty array as missing) by using a fallback
default array or fallback value: choose pageSizeOptions if it has length and
includes(props.pageSize) ? props.pageSize : pageSizeOptions[0], otherwise fall
back to props.pageSize if valid or a hardcoded default (e.g., 10); ensure
references to pageSizeOptions, props.pageSize, and effectivePageSize are
adjusted so effectivePageSize is never undefined before rendering the Select and
before calling onPageSizeChange.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitlab-ci.yml:
- Around line 37-53: The pipeline uses a non-unique SHORT_TAG
(CI_COMMIT_SHORT_SHA) for image tagging and may publish :latest concurrently;
change SHORT_TAG to use the full CI_COMMIT_SHA (i.e. set
SHORT_TAG="${CI_COMMIT_SHA}" or replace usages of SHORT_TAG with CI_COMMIT_SHA
when building/taging) so image tags are content-addressable, and add a
resource_group to the job that retags/pushes the latest image (the block that
checks CI_COMMIT_BRANCH == CI_DEFAULT_BRANCH and runs docker tag/push for
:latest) so publication of latest is serialized across concurrent pipelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| - REF_TAG="${CI_COMMIT_REF_SLUG}" | ||
| - SHORT_TAG="${CI_COMMIT_SHORT_SHA}" | ||
| - | | ||
| docker build \ | ||
| --platform linux/amd64 \ | ||
| -f docker/dockerfile/Dockerfile \ | ||
| --target prod \ | ||
| -t "${CI_REGISTRY_IMAGE}:${REF_TAG}" \ | ||
| -t "${CI_REGISTRY_IMAGE}:${SHORT_TAG}" \ | ||
| . | ||
| - docker push "${CI_REGISTRY_IMAGE}:${REF_TAG}" | ||
| - docker push "${CI_REGISTRY_IMAGE}:${SHORT_TAG}" | ||
| - | | ||
| if [ "${CI_COMMIT_BRANCH:-}" = "${CI_DEFAULT_BRANCH}" ]; then | ||
| docker tag "${CI_REGISTRY_IMAGE}:${REF_TAG}" "${CI_REGISTRY_IMAGE}:latest" | ||
| docker push "${CI_REGISTRY_IMAGE}:latest" | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitLab CI docs: Is CI_COMMIT_SHORT_SHA guaranteed unique across repository history? What is the recommended tag for immutable Docker images (CI_COMMIT_SHA), and what is the recommended way to prevent concurrent pipelines from overwriting latest (e.g., resource_group)?
💡 Result:
-
Is
CI_COMMIT_SHORT_SHAguaranteed unique? No. GitLab defines it as the first 8 characters ofCI_COMMIT_SHA[1], and GitLab explicitly notes short SHAs are a convenient reference but “not guaranteed unique” [2]. Use the full SHA when uniqueness matters. -
Recommended tag for immutable Docker images: Tag images with the full commit SHA (
$CI_COMMIT_SHA) so each build is content-addressable/traceable to an exact commit. GitLab’s own container-scanning convention publishes images using$CI_COMMIT_SHAin the image name/tag [3]. -
Prevent concurrent pipelines from overwriting
latest:- Serialize the “push
latest” job withresource_groupso only one pipeline at a time can run that critical section (mutual exclusion across pipelines) [4].push_latest: stage: deploy resource_group: docker-latest script: - docker push "$CI_REGISTRY_IMAGE:latest"
- For deployments, also enable GitLab’s “Prevent outdated deployment jobs” setting to stop older pipelines from finishing later and overwriting a newer “latest” deployment [5].
- Serialize the “push
Use immutable tags and serialize latest publication.
Line 38 uses CI_COMMIT_SHORT_SHA, which is not guaranteed to be unique across your repository's history (GitLab defines it as the first 8 characters of the full SHA), and Lines 50–53 can publish latest out of order when two default-branch pipelines overlap.
🔧 Proposed fix
build:
stage: build
image: docker:27-cli
+ resource_group: registry-latest-publish
services:
- docker:27-dind
@@
- - SHORT_TAG="${CI_COMMIT_SHORT_SHA}"
+ - SHA_TAG="${CI_COMMIT_SHA}"
@@
-t "${CI_REGISTRY_IMAGE}:${REF_TAG}" \
- -t "${CI_REGISTRY_IMAGE}:${SHORT_TAG}" \
+ -t "${CI_REGISTRY_IMAGE}:${SHA_TAG}" \
.
- docker push "${CI_REGISTRY_IMAGE}:${REF_TAG}"
- - docker push "${CI_REGISTRY_IMAGE}:${SHORT_TAG}"
+ - docker push "${CI_REGISTRY_IMAGE}:${SHA_TAG}"Use the full CI_COMMIT_SHA for content-addressable image tags, and add resource_group to serialize the latest publication across concurrent pipelines.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitlab-ci.yml around lines 37 - 53, The pipeline uses a non-unique
SHORT_TAG (CI_COMMIT_SHORT_SHA) for image tagging and may publish :latest
concurrently; change SHORT_TAG to use the full CI_COMMIT_SHA (i.e. set
SHORT_TAG="${CI_COMMIT_SHA}" or replace usages of SHORT_TAG with CI_COMMIT_SHA
when building/taging) so image tags are content-addressable, and add a
resource_group to the job that retags/pushes the latest image (the block that
checks CI_COMMIT_BRANCH == CI_DEFAULT_BRANCH and runs docker tag/push for
:latest) so publication of latest is serialized across concurrent pipelines.
|
@romzes5000, is this PR ready to be reviewed, or does more work still need to be done? |
2 participants
Summary
Adds a Backup button to the project detail page header that queues backup jobs for all databases in the project at once. Databases that already have a backup waiting or in progress are automatically skipped.
Motivation
When managing projects with many databases, triggering backups one by one is tedious. This feature lets project admins/owners start a backup for every database in a single click — the same workflow as the existing per-database "Backup" button, but at the project level.
Changes
src/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-project.action.tswaitingbackup rows inside a single transactionsrc/components/wrappers/dashboard/projects/mass-backup-button/mass-backup-button.tsxButtonWithConfirm(same pattern asBackupButton)app/(customer)/dashboard/(organization)/projects/[projectId]/page.tsxMassBackupButtonto the project toolbar (visible to owner/admin only)Design decisions
DatabaseZap), confirmation dialog copy ("Create Backup" / "Yes, create backup" / "No, cancel"), and toast pattern as the existing single-databaseBackupButtonmemberrole; unified error message ("Project not found or access denied") prevents project enumerationTest plan
memberrole → verify the button is not visiblewaitingrows are createdSummary by CodeRabbit