fix: add page size selector to CardsWithPagination#253
fix: add page size selector to CardsWithPagination#253RambokDev merged 4 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>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThese changes introduce configurable page size selection to the pagination component system. Two project listing pages now support user-selectable pagination sizes (10/20/50 items and 12/24/48 items respectively), while the core pagination component and a new dropdown UI control enable this functionality. Changes
Poem
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 `@src/components/wrappers/common/cards-with-pagination.tsx`:
- Line 28: The pagination math uses totalPages = Math.ceil(data.length /
pageSize) but pageSize updates (e.g., in the onPageSizeChange handler) are not
validated; ensure you guard updates so pageSize can never be 0, negative, or NaN
before recomputing totalPages. Modify the logic around the pageSize setter /
handler (where newSize is accepted, referenced in the onPageSizeChange /
setPageSize code) to coerce/validate newSize (e.g., require newSize >= 1 and
isFinite) and only apply it if valid, otherwise fall back to a default (like 1)
or ignore the update, then recalc totalPages using the validated pageSize.
- Around line 66-73: PaginationNavigation is missing the className on the
"no-selector" rendering path, causing it to revert to a centered/full-width
layout; update the no-selector branch in cards-with-pagination.tsx to pass the
same className used in the selector path to <PaginationNavigation> (the same
alignment container class used where pageSizeOptions is present) so pagination
keeps its previous alignment and layout when pageSizeOptions is not provided.
- Around line 59-65: The page-size selector is being rendered even when
pageSizeOptions is an empty array because the check uses `pageSizeOptions &&`,
which is truthy for `[]`; update the render gate to only render `PaginationSize`
when `pageSizeOptions.length > 0` (i.e., replace the current conditional that
references `pageSizeOptions` with a length check) so the `PaginationSize`
component (props: `pageSize`, `onPageSizeChange`/`handlePageSizeChange`,
`pageSizeOptions`) is only mounted when there are actual options.
In `@src/components/wrappers/common/pagination/pagination-size.tsx`:
- Around line 16-23: The Select trigger can become unlabeled when the visible
label is hidden on small screens; update the page-size control (the Select /
SelectTrigger / SelectValue usage and onPageSizeChange handler) to provide an
explicit accessible name by adding an aria-label on SelectTrigger (e.g., "Cards
per page") or by inserting a persistent screen-reader-only label (sr-only)
associated with the Select, so assistive tech always has a clear name for the
control.
🪄 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: 63bf2e3c-973d-4afa-929e-d123f8e8b840
📒 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 (2)
app/(customer)/dashboard/(organization)/projects/page.tsx (1)
69-71: Good call-site wiring for configurable page size.The default (
12) is included inpageSizeOptions, so initial render and selector state stay consistent.app/(customer)/dashboard/(organization)/projects/[projectId]/page.tsx (1)
105-107: Looks good: page-size configuration is consistent.
cardsPerPage={20}matches one of the provided options, so the selector and pagination initialize predictably.
- 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>
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/common/pagination/pagination-size.tsx`:
- Around line 17-22: The Select may show a value for pageSize that isn't present
in pageSizeOptions, causing no option to appear selected; update the component
to validate that the incoming pageSize exists in pageSizeOptions (check array
membership) and if not, set a sensible fallback (e.g., default to
pageSizeOptions[0] or null) before passing it to Select's value and SelectValue,
and call onPageSizeChange with the fallback so the dropdown state and selection
stay consistent; optionally emit a console.warn or use the component logger when
pageSize is invalid to help future debugging.
🪄 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: 55539b8b-88b9-4aa9-8e96-808e67726ef7
📒 Files selected for processing (2)
src/components/wrappers/common/cards-with-pagination.tsxsrc/components/wrappers/common/pagination/pagination-size.tsx
📜 Review details
🔇 Additional comments (4)
src/components/wrappers/common/cards-with-pagination.tsx (2)
46-52: LGTM! Validation and guard logic properly implemented.The
handlePageSizeChangefunction correctly validates input (rejects non-finite, zero, or negative values), andshowSizeSelectorproperly guards against empty arrays. Both address prior review feedback.
61-78: LGTM! Layout and backward compatibility preserved.The flex container with
justify-endalignment and conditional rendering ofPaginationSizeis well-structured.PaginationNavigationnow receivesclassName="justify-end"which maintains layout consistency whether or not the size selector is shown.src/components/wrappers/common/pagination/pagination-size.tsx (2)
21-21: LGTM! Accessibility label properly added.The
aria-label="Cards per page"onSelectTriggerensures the control remains accessible when the visible text label is hidden on smaller screens.
11-33: Clean, well-structured component.The component follows good patterns: controlled Select with proper value coercion, responsive layout, accessible labeling, and sensible defaults. The integration with Radix UI Select is correct.
…t in options Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/wrappers/common/pagination/pagination-size.tsx (1)
13-20:⚠️ Potential issue | 🟠 MajorFallback page size is not synchronized back to parent state
At
Line 13, the component computeseffectivePageSize, but atLine 20it only updates parent state on manual selection. If incomingprops.pageSizeis invalid, UI shows fallback while pagination logic can still use stale parent state.💡 Proposed fix
+import { useEffect } from "react"; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; import { cn } from "@/lib/utils"; @@ 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]; + const { className, pageSize, onPageSizeChange, pageSizeOptions = [10, 20, 30, 40, 50] } = props; + const hasValidPageSize = pageSizeOptions.includes(pageSize); + const effectivePageSize = hasValidPageSize ? pageSize : pageSizeOptions[0]; + + useEffect(() => { + if (!hasValidPageSize && pageSizeOptions.length > 0) { + onPageSizeChange(pageSizeOptions[0]); + } + }, [hasValidPageSize, onPageSizeChange, pageSizeOptions]); @@ - value={`${effectivePageSize}`} + value={`${effectivePageSize}`} onValueChange={(value) => onPageSizeChange(Number(value))}#!/bin/bash set -euo pipefail rg -n 'effectivePageSize|onValueChange|useState\(cardsPerPage\)|handlePageSizeChange' \ src/components/wrappers/common/pagination/pagination-size.tsx \ src/components/wrappers/common/cards-with-pagination.tsx🤖 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 13 - 20, The component computes effectivePageSize from props.pageSize but never syncs that fallback back to the parent; add a sync so parent state is updated when props.pageSize is invalid. Specifically, in pagination-size.tsx detect when pageSizeOptions.includes(props.pageSize) is false (i.e. effectivePageSize !== props.pageSize) and call onPageSizeChange(effectivePageSize) (or Number(effectivePageSize) to match types) — implement this in a useEffect that runs when props.pageSize or pageSizeOptions change so the parent is updated immediately rather than only on Select onValueChange.
🤖 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/common/pagination/pagination-size.tsx`:
- Line 23: The SelectValue placeholder is using an undefined identifier pageSize
which breaks compilation; update SelectValue to use the already-computed
effectivePageSize instead (replace any occurrence of pageSize in the SelectValue
placeholder with effectivePageSize) so the component (which destructures
className, onPageSizeChange, pageSizeOptions) compiles correctly and uses the
correct prop-derived value.
---
Duplicate comments:
In `@src/components/wrappers/common/pagination/pagination-size.tsx`:
- Around line 13-20: The component computes effectivePageSize from
props.pageSize but never syncs that fallback back to the parent; add a sync so
parent state is updated when props.pageSize is invalid. Specifically, in
pagination-size.tsx detect when pageSizeOptions.includes(props.pageSize) is
false (i.e. effectivePageSize !== props.pageSize) and call
onPageSizeChange(effectivePageSize) (or Number(effectivePageSize) to match
types) — implement this in a useEffect that runs when props.pageSize or
pageSizeOptions change so the parent is updated immediately rather than only on
Select onValueChange.
🪄 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: 83d1c7a8-b41d-4123-b446-65de2ffefcf4
📒 Files selected for processing (1)
src/components/wrappers/common/pagination/pagination-size.tsx
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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:
- Line 8: IMAGE_LATEST is being set to $CI_REGISTRY_IMAGE:latest which is then
pushed for non-default branches; restrict publishing the mutable :latest tag to
the default branch only. Update the pipeline so IMAGE_LATEST is only
created/pushed when the commit is on $CI_DEFAULT_BRANCH (e.g., wrap the
IMAGE_LATEST assignment or the jobs that push $IMAGE_LATEST with rules/only: if:
'$CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH'), and ensure branch builds continue to
publish the immutable SHA tag (the job that uses the per-commit
IMAGE_TAG/$CI_COMMIT_SHA remains unchanged).
- Around line 39-41: The deploy job currently targets the production environment
(environment: name: production) and uses PORTABASE_HOST while its rules allow
manual runs from feature branches (e.g., feat/cards-pagination-size-selector);
change the rules so production deployments are only allowed from the default
branch by replacing the branch check with $CI_DEFAULT_BRANCH (or add a condition
requiring $CI_DEFAULT_BRANCH) and/or add a separate staging environment route
for non-default branches so only the job matched against $CI_DEFAULT_BRANCH can
deploy to production; update the job's rules block that references branch names
and when: manual accordingly.
- Line 55: The sed command in .gitlab-ci.yml currently replaces every "image:"
entry in docker-compose.yml; change it to only update the app service's image by
targeting the app service block (e.g., match the "services:" → "app:" section
and replace only the subsequent "image:" line) or switch to a structured tool
(yq) or a dedicated placeholder in docker-compose.yml; update the sed invocation
that references $IMAGE so it only alters the image line under the app service
(not global "image:" entries) and keep the variable $IMAGE as the replacement
value.
- Line 17: Replace direct password args to docker login with stdin-based login
calls: where you currently have the literal command "docker login -u
$CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD $CI_REGISTRY" and the similar
SSH-block login, change them to use echo "$CI_REGISTRY_PASSWORD" | docker login
--username "$CI_REGISTRY_USER" --password-stdin "$CI_REGISTRY"; for the SSH
block, pipe the variable into docker login inside the SSH session (or use a
quoted heredoc) so the password is not passed on the command line.
In `@src/components/wrappers/common/pagination/pagination-size.tsx`:
- Around line 11-14: Guard against an empty pageSizeOptions by creating a safe
fallback array and using it everywhere instead of the raw prop: compute const
safePageSizeOptions = pageSizeOptions && pageSizeOptions.length ?
pageSizeOptions : [10,20,30,40,50], then base effectivePageSize on
safePageSizeOptions (e.g. const effectivePageSize =
safePageSizeOptions.includes(props.pageSize) ? props.pageSize :
safePageSizeOptions[0]) and use safePageSizeOptions when rendering the dropdown
and when handling onPageSizeChange; this prevents effectivePageSize becoming
undefined and avoids rendering "undefined" options.
🪄 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: 59c03da9-10d2-42ee-ab3a-569f71f09f30
📒 Files selected for processing (2)
.gitlab-ci.ymlsrc/components/wrappers/common/pagination/pagination-size.tsx
📜 Review details
🔇 Additional comments (3)
src/components/wrappers/common/pagination/pagination-size.tsx (3)
1-2: LGTM!Imports are clean and follow the project's internal component and utility conventions.
4-9: LGTM!The type definition is clear and well-structured with appropriate required/optional distinctions.
15-34: LGTM!The JSX is well-structured:
- Accessibility is properly handled with
aria-labelon the trigger for screen readers when the visible label is hidden.- The
effectivePageSizeis correctly used for both the controlledvalueand placeholder.- The Select options are cleanly mapped with appropriate keys.
.gitlab-ci.yml
Outdated
| variables: | ||
| DOCKER_TLS_CERTDIR: "" | ||
| IMAGE: $CI_REGISTRY_IMAGE:$CI_COMMIT_SHORT_SHA | ||
| IMAGE_LATEST: $CI_REGISTRY_IMAGE:latest |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name ".gitlab-ci.yml" -type fRepository: Portabase/portabase
Length of output: 152
🏁 Script executed:
cat -n .gitlab-ci.ymlRepository: Portabase/portabase
Length of output: 2196
Only publish :latest from the default branch.
Lines 25-26 push $IMAGE_LATEST, and Lines 29-30 allow that to happen on feat/cards-pagination-size-selector. This makes the registry's latest tag point at unmerged code. Keep branch builds on the immutable SHA tag and reserve latest for the default branch only.
Restrict `latest` to the default branch
- docker build
--platform linux/amd64
-f docker/dockerfile/Dockerfile
--target prod
-t $IMAGE
- -t $IMAGE_LATEST
.
- docker push $IMAGE
- - docker push $IMAGE_LATEST
- - docker rmi $IMAGE $IMAGE_LATEST || true
+ - |
+ if [ "$CI_COMMIT_BRANCH" = "$CI_DEFAULT_BRANCH" ]; then
+ docker tag "$IMAGE" "$IMAGE_LATEST"
+ docker push "$IMAGE_LATEST"
+ docker rmi "$IMAGE_LATEST" || true
+ fi
+ - docker rmi "$IMAGE" || true📝 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.
| IMAGE_LATEST: $CI_REGISTRY_IMAGE:latest | |
| - docker build | |
| --platform linux/amd64 | |
| -f docker/dockerfile/Dockerfile | |
| --target prod | |
| -t $IMAGE | |
| . | |
| - docker push $IMAGE | |
| - | | |
| if [ "$CI_COMMIT_BRANCH" = "$CI_DEFAULT_BRANCH" ]; then | |
| docker tag "$IMAGE" "$IMAGE_LATEST" | |
| docker push "$IMAGE_LATEST" | |
| docker rmi "$IMAGE_LATEST" || true | |
| fi | |
| - docker rmi "$IMAGE" || true |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitlab-ci.yml at line 8, IMAGE_LATEST is being set to
$CI_REGISTRY_IMAGE:latest which is then pushed for non-default branches;
restrict publishing the mutable :latest tag to the default branch only. Update
the pipeline so IMAGE_LATEST is only created/pushed when the commit is on
$CI_DEFAULT_BRANCH (e.g., wrap the IMAGE_LATEST assignment or the jobs that push
$IMAGE_LATEST with rules/only: if: '$CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH'),
and ensure branch builds continue to publish the immutable SHA tag (the job that
uses the per-commit IMAGE_TAG/$CI_COMMIT_SHA remains unchanged).
.gitlab-ci.yml
Outdated
| - oxem-hetzner | ||
| image: docker:cli | ||
| script: | ||
| - docker login -u $CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD $CI_REGISTRY |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .gitlab-ci.ymlRepository: Portabase/portabase
Length of output: 2196
🌐 Web query:
docker login --password-stdin vs -p security best practices
💡 Result:
docker login --password-stdin vs -p/--password: security differences
-p/--passwordis riskier because the secret is passed as a command-line argument, which can end up exposed via tooling and automation (for example in logs). Docker’s own guidance is to prefer stdin for non-interactive use. [1]--password-stdinis preferred because it feeds the secret via STDIN, which helps prevent it from ending up in shell history or log files. [1]- Typical pattern:
printf %s "$TOKEN" | docker login -u "$USER" --password-stdin REGISTRY[1][2]
- Typical pattern:
Important caveat: --password-stdin doesn’t solve credential-at-rest risk
Even if you use stdin safely, Docker may still store credentials locally:
- If you don’t configure a credential store/helper, Docker stores credentials in
~/.docker/config.jsonin base64-encoded form (not strong protection). [1] - Docs from cloud providers similarly note Docker’s
config.jsonstorage behavior and recommend credential helpers. [3]
Best practices (recommended)
- Use
--password-stdin, not-p. [1][2] - Use a credential store / credential helper (OS keychain,
pass, Secret Service, etc.) instead of file-based storage. [1][3] - Use tokens (PATs) instead of account passwords where supported/required (Docker increasingly emphasizes PAT-based auth, especially with SSO enforcement scenarios). [1][4]
- In CI, source the token from a secret manager and pipe it to
docker login --password-stdin(avoid echoing secrets to logs). [1][2]
Sources: Docker CLI docker login docs [1], GitLab registry auth docs [2], Google Artifact Registry Docker auth docs [3], Docker blog on password-login deprecation / PATs with SSO enforcement [4].
🌐 Web query:
GitLab CI registry password masking CI_REGISTRY_PASSWORD
💡 Result:
CI_REGISTRY_PASSWORD is a predefined GitLab CI/CD variable used to authenticate to the project’s Container Registry, and its value is the same as CI_JOB_TOKEN (valid only while the job is running). (docs.gitlab.com)
Why you see [MASKED]
GitLab masks job tokens in job logs. Since CI_REGISTRY_PASSWORD is CI_JOB_TOKEN, it will be replaced with [MASKED] when printed (for example, via echo, set -x, verbose tooling output, etc.). (docs.gitlab.com)
What to do
Use it without printing it, e.g. for Docker login:
printf "%s" "$CI_REGISTRY_PASSWORD" | docker login -u "$CI_REGISTRY_USER" --password-stdin "$CI_REGISTRY"(CI_REGISTRY_USER / CI_REGISTRY_PASSWORD are intended for registry auth in jobs.) (docs.gitlab.com)
If you need a long-lived registry password
CI_REGISTRY_PASSWORD is job-scoped; for longer-lived access, use a Deploy Token (exposed in CI as CI_DEPLOY_USER / CI_DEPLOY_PASSWORD). (docs.gitlab.com)
Citations:
- 1: https://docs.gitlab.com/ci/variables/predefined_variables/
- 2: https://docs.gitlab.com/ci/jobs/ci_job_token/
- 3: https://docs.gitlab.com/ci/variables/predefined_variables/
- 4: https://docs.gitlab.com/ci/variables/predefined_variables/
Use --password-stdin for both registry logins.
Lines 17 and 52 pass $CI_REGISTRY_PASSWORD on the command line. While GitLab CI masks this variable in logs, the password is still exposed via process arguments where it can be captured by monitoring tools and local utilities. Switch both logins to --password-stdin: on line 17 directly, and on line 52 within the SSH block using a quoted heredoc or explicit variable passing.
Safer local login command
- - docker login -u $CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD $CI_REGISTRY
+ - printf '%s' "$CI_REGISTRY_PASSWORD" | docker login -u "$CI_REGISTRY_USER" --password-stdin "$CI_REGISTRY"Also applies to: 49-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitlab-ci.yml at line 17, Replace direct password args to docker login with
stdin-based login calls: where you currently have the literal command "docker
login -u $CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD $CI_REGISTRY" and the
similar SSH-block login, change them to use echo "$CI_REGISTRY_PASSWORD" |
docker login --username "$CI_REGISTRY_USER" --password-stdin "$CI_REGISTRY"; for
the SSH block, pipe the variable into docker login inside the SSH session (or
use a quoted heredoc) so the password is not passed on the command line.
.gitlab-ci.yml
Outdated
| environment: | ||
| name: production | ||
| url: https://portabase.oxem.dev |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name ".gitlab-ci.yml" -type f | head -5Repository: Portabase/portabase
Length of output: 152
🏁 Script executed:
cat -n ./.gitlab-ci.ymlRepository: Portabase/portabase
Length of output: 2196
Block production deploys from the feature branch.
The deploy job is marked as the production environment (lines 39–41) and executes against the live production host (line 9: PORTABASE_HOST: "159.194.216.86", used at line 50). However, the rules (lines 59–63) allow this job to run on feat/cards-pagination-size-selector as well as main, both with when: manual. This permits any maintainer to manually trigger a production deployment from an unmerged feature branch.
Restrict this job to the default branch using $CI_DEFAULT_BRANCH, or route non-main branches to a separate staging environment.
Safer production rule set
rules:
- - if: $CI_COMMIT_BRANCH == "feat/cards-pagination-size-selector"
- when: manual
- - if: $CI_COMMIT_BRANCH == "main"
+ - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH
when: manual🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitlab-ci.yml around lines 39 - 41, The deploy job currently targets the
production environment (environment: name: production) and uses PORTABASE_HOST
while its rules allow manual runs from feature branches (e.g.,
feat/cards-pagination-size-selector); change the rules so production deployments
are only allowed from the default branch by replacing the branch check with
$CI_DEFAULT_BRANCH (or add a condition requiring $CI_DEFAULT_BRANCH) and/or add
a separate staging environment route for non-default branches so only the job
matched against $CI_DEFAULT_BRANCH can deploy to production; update the job's
rules block that references branch names and when: manual accordingly.
.gitlab-ci.yml
Outdated
| docker login -u $CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD $CI_REGISTRY | ||
| docker pull $IMAGE | ||
| cd /root/portabase | ||
| sed -i "s|image:.*|image: $IMAGE|" docker-compose.yml |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find docker-compose files in the repo
git ls-files | grep -E 'docker-compose|\.gitlab-ci'Repository: Portabase/portabase
Length of output: 167
🏁 Script executed:
# Check directory structure
ls -la | head -20Repository: Portabase/portabase
Length of output: 1273
🏁 Script executed:
# View .gitlab-ci.yml to see the context around line 55
cat -n .gitlab-ci.yml | sed -n '45,65p'Repository: Portabase/portabase
Length of output: 874
🏁 Script executed:
# Examine docker-compose.yml for image entries
echo "=== docker-compose.yml ===" && cat -n docker-compose.yml | grep -C 2 "image:"Repository: Portabase/portabase
Length of output: 313
🏁 Script executed:
# Examine docker-compose.prod.yml for image entries
echo "=== docker-compose.prod.yml ===" && cat -n docker-compose.prod.yml | grep -C 2 "image:"Repository: Portabase/portabase
Length of output: 377
🏁 Script executed:
# Get the full docker-compose.yml to see all services
cat -n docker-compose.ymlRepository: Portabase/portabase
Length of output: 1108
🏁 Script executed:
# Check if there's an app service in docker-compose.yml
grep -n "^\s*[a-z].*:" docker-compose.yml | head -20Repository: Portabase/portabase
Length of output: 484
🏁 Script executed:
# Get full context of docker-compose.prod.yml to understand the structure
cat -n docker-compose.prod.ymlRepository: Portabase/portabase
Length of output: 1158
Scope the sed replacement to the app service image only.
Line 55 replaces every image: entry in docker-compose.yml. The file contains multiple services with image specifications (db at line 5, tusd at line 21), so the sed pattern will overwrite all of them with $IMAGE, breaking the database and file upload services. This would prevent docker compose up -d from functioning. Restrict the replacement to only update the app service's image, preferably by adding an explicit app service entry to docker-compose.yml with a structured edit or placeholder.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitlab-ci.yml at line 55, The sed command in .gitlab-ci.yml currently
replaces every "image:" entry in docker-compose.yml; change it to only update
the app service's image by targeting the app service block (e.g., match the
"services:" → "app:" section and replace only the subsequent "image:" line) or
switch to a structured tool (yq) or a dedicated placeholder in
docker-compose.yml; update the sed invocation that references $IMAGE so it only
alters the image line under the app service (not global "image:" entries) and
keep the variable $IMAGE as the replacement value.
| 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 a caller passes an empty array pageSizeOptions={[]}, pageSizeOptions[0] evaluates to undefined, causing effectivePageSize to be undefined. This would render "undefined" in the dropdown and produce no selectable items.
Consider adding a guard or early return when the options array is empty:
Proposed fix
export const PaginationSize = (props: PaginationSizeProps) => {
const { className, onPageSizeChange, pageSizeOptions = [10, 20, 30, 40, 50] } = props;
+
+ if (pageSizeOptions.length === 0) {
+ return null;
+ }
+
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 = [10, 20, 30, 40, 50] } = props; | |
| if (pageSizeOptions.length === 0) { | |
| return null; | |
| } | |
| 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, Guard against an empty pageSizeOptions by creating a safe fallback
array and using it everywhere instead of the raw prop: compute const
safePageSizeOptions = pageSizeOptions && pageSizeOptions.length ?
pageSizeOptions : [10,20,30,40,50], then base effectivePageSize on
safePageSizeOptions (e.g. const effectivePageSize =
safePageSizeOptions.includes(props.pageSize) ? props.pageSize :
safePageSizeOptions[0]) and use safePageSizeOptions when rendering the dropdown
and when handling onPageSizeChange; this prevents effectivePageSize becoming
undefined and avoids rendering "undefined" options.
|
Hi @romzes5000, Thanks for the PR, we will review it as soon as possible. It appears that you pushed your GitLab CI file with sensitive information. Could you remove that? Could you also describe more precisely the purpose of this PR? Thanks for that. If you have any questions or suggestions, feel free to join our Discord. |
romzes5000
force-pushed
the
feat/cards-pagination-size-selector
branch
from
93dd96a to
cd92135
Compare
|
Hi @RambokDev, Thanks for catching that! I have already removed the Regarding the purpose: when managing projects with many databases, the current fixed page size (6 cards) forces excessive pagination. This PR adds an optional |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/wrappers/common/pagination/pagination-size.tsx (1)
12-14:⚠️ Potential issue | 🟡 MinorHandle empty
pageSizeOptionsto avoid invalid select state.If
pageSizeOptions={[]}is passed,effectivePageSizebecomesundefined(Line 13), so the select value becomes"undefined"and the dropdown has no items. Use a safe fallback array before computingeffectivePageSize.Suggested fix
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]; + const safePageSizeOptions = pageSizeOptions.length > 0 ? pageSizeOptions : [10, 20, 30, 40, 50]; + const effectivePageSize = safePageSizeOptions.includes(props.pageSize) ? props.pageSize : safePageSizeOptions[0]; @@ - {pageSizeOptions.map((size) => ( + {safePageSizeOptions.map((size) => ( <SelectItem key={size} value={`${size}`}> {size} </SelectItem> ))}🤖 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 12 - 14, The code computes effectivePageSize from pageSizeOptions which can be an empty array, causing effectivePageSize to become undefined and the select to enter an invalid state; fix by normalizing pageSizeOptions to a safeFallback (e.g., default [10,20,30,40,50]) before computing effectivePageSize so that const safeOptions = (pageSizeOptions && pageSizeOptions.length) ? pageSizeOptions : [10,20,30,40,50]; then compute effectivePageSize = safeOptions.includes(props.pageSize) ? props.pageSize : safeOptions[0] and use safeOptions for rendering and onPageSizeChange handling (referencing props.pageSize, pageSizeOptions, effectivePageSize, and onPageSizeChange).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/wrappers/common/pagination/pagination-size.tsx`:
- Around line 12-14: The code computes effectivePageSize from pageSizeOptions
which can be an empty array, causing effectivePageSize to become undefined and
the select to enter an invalid state; fix by normalizing pageSizeOptions to a
safeFallback (e.g., default [10,20,30,40,50]) before computing effectivePageSize
so that const safeOptions = (pageSizeOptions && pageSizeOptions.length) ?
pageSizeOptions : [10,20,30,40,50]; then compute effectivePageSize =
safeOptions.includes(props.pageSize) ? props.pageSize : safeOptions[0] and use
safeOptions for rendering and onPageSizeChange handling (referencing
props.pageSize, pageSizeOptions, effectivePageSize, and onPageSizeChange).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 80451e04-b52a-4cf9-bccc-1e82d6c2417c
📒 Files selected for processing (1)
src/components/wrappers/common/pagination/pagination-size.tsx
|
Hi @romzes5000, The PR looks good, nothing to flag. I tested it. For a future PR, it would be useful to persist the default car pagination in the default settings or user model, so it doesn’t need to be reset each time you access or reload the page. Feel free to open a PR for this. It would require adding a new field either in the default settings or at the user level, to avoid impacting the entire system. Let me know if you have any questions about this |
RambokDev
changed the title
|
Hi @RambokDev — thanks for the note on persisting the cards page size. For the follow-up PR I see two options:
I'd lean toward (2) if cross-device consistency matters, and (1) if we want the smallest possible first step. Which approach do you prefer? Happy to implement either. |
The second approach will be perfect for a long-term scope. |
2 participants
Summary
pageSizeOptionsprop toCardsWithPaginationthat renders a "Cards per page" dropdown selectorPaginationSizecomponent — standalone, consistent with existingTablePaginationSizepattern but without TanStack Table dependencypageSizeOptions, behavior is unchangedMotivation
When managing many databases in a project, the current limit of 6 cards per page requires excessive pagination. The
DataTablecomponent already offers a page size selector — this PR brings the same UX to card-based views.Changes
src/components/wrappers/common/pagination/pagination-size.tsxsrc/components/wrappers/common/cards-with-pagination.tsxpageSizeOptionsprop, managepageSizeas stateapp/(customer)/dashboard/(organization)/projects/[projectId]/page.tsxcardsPerPage={20},pageSizeOptions={[10, 20, 50]}app/(customer)/dashboard/(organization)/projects/page.tsxcardsPerPage={12},pageSizeOptions={[12, 24, 48]}Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit