feat: implement redesigned dormitories overview page with CMS integra…#170
Hidden character warning
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a new Dormitories Payload CMS collection with a database migration and generated types, a ChangesDormitories Feature
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Page as DormitoriesOverviewPage
participant Dict as getDictionary
participant CMS as getDormitories
participant ImageCard
Page->>Dict: getDictionary(lang, "dormitories")
Page->>CMS: getDormitories()
CMS-->>Page: Dormitory[]
Dict-->>Page: dictionary
Page->>Page: map dormitories to cards (or fallbackDormitories)
Page->>ImageCard: render cards with title, href, imageSrc, detailsLabel
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/migrations/20260703_140815_dormitories_collection.ts (1)
9-9: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSame NOT NULL vs.
ON DELETE SET NULLconflict as flagged inDormitories.ts.
cover_image_idisNOT NULL(Line 9) but the FK constraint (Line 17) isON DELETE set null, which will fail with a constraint violation the first time a referencedMediadoc is deleted. See detailed comment onsrc/collections/Dormitories.ts(coverImage field) for the root cause and suggested fixes.Also applies to: 17-17
🤖 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 `@src/migrations/20260703_140815_dormitories_collection.ts` at line 9, The migration for the dormitories collection has the same nullability mismatch as the Dormitories collection definition: cover_image_id is declared NOT NULL while the foreign key uses ON DELETE SET NULL. Update the schema in this migration so the column nullability matches the FK behavior, or change the FK action to match the non-null constraint; use the cover_image_id column and its FK constraint in the dormitories migration as the reference points.
🧹 Nitpick comments (1)
src/collections/Dormitories.ts (1)
38-47: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd slug format validation.
slughas no format constraint, yet it's interpolated directly into route paths downstream (/${lang}/kollegium/kollegium-bemutato/${dormitory.slug}). An admin entering spaces, slashes, or other special characters would produce broken/unexpected URLs.♻️ Suggested validation
{ name: "slug", label: "Slug", type: "text", required: true, unique: true, admin: { description: "Útvonalhoz használt azonosító, pl. baross, sch.", }, + validate: (val: string | null | undefined) => { + if (!val) return "A slug megadása kötelező."; + return /^[a-z0-9]+(?:-[a-z0-9]+)*$/.test(val) + ? true + : "A slug csak kisbetűket, számokat és kötőjeleket tartalmazhat."; + }, },🤖 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 `@src/collections/Dormitories.ts` around lines 38 - 47, Add format validation for the slug field in Dormitories so only URL-safe values are accepted. Update the slug field definition in the Dormitories collection to enforce a slug-style pattern (for example lowercase letters, numbers, and hyphens only), and keep it aligned with how dormitory.slug is later interpolated into route paths. Use the existing slug field configuration in Dormitories as the place to add the validation so invalid values with spaces, slashes, or special characters are rejected before save.
🤖 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 `@src/app/`(app)/[lang]/kollegium/kollegium-bemutato/page.tsx:
- Around line 80-111: The empty-state branch in page.tsx is unreachable because
renderedCards always falls back to fallbackDormitories, so d.no_results can
never render. Update the logic around renderedCards and the conditional render
in the kollegium-bemutato page so the empty state is driven by the actual CMS
result (cards.length) or otherwise avoid using the always-nonempty fallback when
deciding whether to show the no_results paragraph.
In `@src/collections/Dormitories.ts`:
- Around line 48-54: The coverImage field in Dormitories is configured as
required while the underlying cover_image_id foreign key uses a delete behavior
that sets null, so adjust one side to match. Update the Dormitories collection
definition around coverImage by either removing required: true so the relation
can be nullable, or change the FK delete action for cover_image_id to
restrict/delete instead of set null. Make sure the chosen behavior is consistent
with the coverImage field and any related media deletion logic.
In `@src/components/common/ImageCard.tsx`:
- Around line 30-38: The ImageCard component uses next/image with a remote media
URL from the Payload S3-backed collection, but the app config does not whitelist
that host. Update the Next.js image configuration in next.config.ts to add the
media origin via images.remotePatterns or images.domains, and make sure the
Image usage in ImageCard still renders remote coverImage.url sources correctly.
---
Duplicate comments:
In `@src/migrations/20260703_140815_dormitories_collection.ts`:
- Line 9: The migration for the dormitories collection has the same nullability
mismatch as the Dormitories collection definition: cover_image_id is declared
NOT NULL while the foreign key uses ON DELETE SET NULL. Update the schema in
this migration so the column nullability matches the FK behavior, or change the
FK action to match the non-null constraint; use the cover_image_id column and
its FK constraint in the dormitories migration as the reference points.
---
Nitpick comments:
In `@src/collections/Dormitories.ts`:
- Around line 38-47: Add format validation for the slug field in Dormitories so
only URL-safe values are accepted. Update the slug field definition in the
Dormitories collection to enforce a slug-style pattern (for example lowercase
letters, numbers, and hyphens only), and keep it aligned with how dormitory.slug
is later interpolated into route paths. Use the existing slug field
configuration in Dormitories as the place to add the validation so invalid
values with spaces, slashes, or special characters are rejected before save.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7d49b82-6654-4b07-aaa8-de9449821126
📒 Files selected for processing (10)
src/app/(app)/[lang]/kollegium/kollegium-bemutato/page.tsxsrc/collections/Dormitories.tssrc/components/common/ImageCard.tsxsrc/dictionaries/en/dormitories.jsonsrc/dictionaries/hu/dormitories.jsonsrc/lib/payload-cms.tssrc/migrations/20260703_140815_dormitories_collection.tssrc/migrations/index.tssrc/payload-types.tssrc/payload.config.ts
| const renderedCards = | ||
| cards.length > 0 | ||
| ? cards | ||
| : fallbackDormitories.map((dormitory) => ({ | ||
| ...dormitory, | ||
| href: `/${lang}/kollegium/kollegium-bemutato/${dormitory.slug}`, | ||
| })); | ||
|
|
||
| return ( | ||
| <div className="min-h-screen bg-gray-50"> | ||
| <div className="min-h-screen bg-[#f9f4f0]"> | ||
| <div className="container mx-auto px-2 md:px-4 py-8"> | ||
| <PageHeader title={dictionary.dormitories.admission_information.introduction} /> | ||
| <DormitoryCardsContainer dormitory={dictionary.dormitories.admission_information.dormitory} linkOrRoute="route"/> | ||
| <PageHeader title={d.introduction} /> | ||
| <div className="rounded-b-2xl border-x border-b border-[#e9e2d6] bg-[#fffefc] p-6 md:p-8"> | ||
| {renderedCards.length > 0 ? ( | ||
| <div className="grid grid-cols-1 gap-4 sm:grid-cols-2 xl:grid-cols-3"> | ||
| {renderedCards.map((dormitory) => ( | ||
| <ImageCard | ||
| key={dormitory.slug} | ||
| content={{ | ||
| href: dormitory.href, | ||
| imageSrc: dormitory.imageSrc, | ||
| title: dormitory.name, | ||
| detailsLabel: d.details, | ||
| }} | ||
| /> | ||
| ))} | ||
| </div> | ||
| ) : ( | ||
| <p className="font-open-sans text-sm leading-[1.6] text-[#6e6660]"> | ||
| {d.no_results} | ||
| </p> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
d.no_results empty-state is unreachable dead code.
Because fallbackDormitories always has 7 entries, renderedCards is guaranteed non-empty whenever cards is empty. The renderedCards.length > 0 check on Line 93 is therefore always true, and the localized no_results paragraph (Lines 107-111) can never render. Either the fallback defeats the intent of the no_results message, or the empty state should key off cards.length (the actual CMS result) instead of renderedCards.
🤖 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 `@src/app/`(app)/[lang]/kollegium/kollegium-bemutato/page.tsx around lines 80 -
111, The empty-state branch in page.tsx is unreachable because renderedCards
always falls back to fallbackDormitories, so d.no_results can never render.
Update the logic around renderedCards and the conditional render in the
kollegium-bemutato page so the empty state is driven by the actual CMS result
(cards.length) or otherwise avoid using the always-nonempty fallback when
deciding whether to show the no_results paragraph.
| { | ||
| name: "coverImage", | ||
| label: "Borítókép", | ||
| type: "upload", | ||
| relationTo: "media", | ||
| required: true, | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does Payload CMS postgres adapter always generate ON DELETE SET NULL for required upload/relationship fields regardless of the required flag?
💡 Result:
No, the Payload CMS Postgres adapter does not unconditionally generate ON DELETE SET NULL for all required relationship fields. The behavior of foreign key constraints in Payload's Postgres adapter has been a subject of ongoing development and is not uniform across all scenarios. Historically, Payload has faced challenges managing database integrity when dealing with required relationships and deletions, leading to various issues where users encounter foreign key constraint violations [1][2][3]. The system does not natively implement automatic cascading deletes for all relationship types, and in many cases, attempting to delete a document that is referenced by a required relationship in another record will result in a database error rather than a soft deletion or nullification [1][3]. Key points regarding this behavior include: 1. Relationship Constraints: There is no global configuration that forces ON DELETE SET NULL. In fact, developers have reported that deleting a document often triggers database-level constraint violations because the foreign key is still active [1][3]. 2. Evolution of Fixes: Payload has introduced specific fixes for certain scenarios, such as adding ON DELETE CASCADE to specific internal table relationships (e.g., select hasMany relationships) to prevent these crashes [4][5]. 3. GraphQL and Nullability: In some contexts, Payload has implemented changes—such as forcing nullability for relationship fields in schema generation—to avoid GraphQL errors when a related document is deleted, which can lead to a conflict between the database schema and the application's strict requirement checks [6]. 4. Recommended Approach: Because there is no simple "required field = automatic set null" behavior, managing deletions that involve required relationships typically requires custom implementation, such as using Payload hooks to manually clean up or cascade delete related records before the primary document is deleted [7][3]. In summary, the adapter does not automatically handle required relationships via ON DELETE SET NULL. Users should expect that deleting a document with active required references may fail with a database error unless handled explicitly through logic in the application layer [1][3].
Citations:
- 1: Postgres: Foreign Key constraint violation after deleting an item payloadcms/payload#10526
- 2: Unable to delete record from admin in postgres if collection has array field type payloadcms/payload#6485
- 3: Unclear error when deleting documents with versioned relationships in SQLite payloadcms/payload#12672
- 4: fix(db-postgres): add
ON DELETE CASCADEto foreign key constraint of select hasMany relationship payloadcms/payload#6735 - 5: payloadcms/payload@9ecc6c8
- 6: payloadcms/payload@560cabe
- 7: Cascade delete payloadcms/payload#655
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files 'src/collections/Dormitories.ts' 'src/migrations/20260703_140815_dormitories_collection.ts'
printf '\n== Dormitories collection outline ==\n'
ast-grep outline src/collections/Dormitories.ts --view expanded || true
printf '\n== Migration outline ==\n'
ast-grep outline src/migrations/20260703_140815_dormitories_collection.ts --view expanded || true
printf '\n== Relevant snippets ==\n'
sed -n '1,140p' src/collections/Dormitories.ts | cat -n
printf '\n--- migration ---\n'
sed -n '1,140p' src/migrations/20260703_140815_dormitories_collection.ts | cat -nRepository: kir-dev/ehk
Length of output: 5459
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'src/collections/Dormitories.ts' 'src/migrations/20260703_140815_dormitories_collection.ts'Repository: kir-dev/ehk
Length of output: 237
Make coverImage nullable or change the FK delete action. cover_image_id is NOT NULL, so ON DELETE SET NULL will fail when a referenced media row is deleted. Either drop required: true or block media deletion while it’s still referenced.
🤖 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 `@src/collections/Dormitories.ts` around lines 48 - 54, The coverImage field in
Dormitories is configured as required while the underlying cover_image_id
foreign key uses a delete behavior that sets null, so adjust one side to match.
Update the Dormitories collection definition around coverImage by either
removing required: true so the relation can be nullable, or change the FK delete
action for cover_image_id to restrict/delete instead of set null. Make sure the
chosen behavior is consistent with the coverImage field and any related media
deletion logic.
| <div className="relative z-10 mx-4 aspect-[349/296] overflow-hidden rounded-2xl border border-[#e9e2d6] bg-[#f9f4f0] sm:mx-8"> | ||
| <Image | ||
| src={imageSrc} | ||
| alt={title} | ||
| fill | ||
| sizes="(max-width: 640px) calc(100vw - 64px), (max-width: 1024px) 45vw, 349px" | ||
| className="object-cover transition-transform duration-300 group-hover:scale-105" | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect Next.js image config for remotePatterns/domains
fd -H -t f 'next.config.*' --exec cat -n {}Repository: kir-dev/ehk
Length of output: 627
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ImageCard =="
fd -H -t f 'ImageCard.tsx' src --exec sh -c 'echo "--- {}"; cat -n "{}"' \;
echo
echo "== overview / imageSrc usage =="
rg -n --context 3 "coverImage|imageSrc|next/image|Image\(" src
echo
echo "== next config / image config references =="
rg -n --context 3 "images:|remotePatterns|domains|withPayload\\(" next.config.* src package.json
echo
echo "== payload config media references =="
rg -n --context 3 "upload|media|s3|cloudinary|url" src/payload.config.ts srcRepository: kir-dev/ehk
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/components/common/ImageCard.tsx =="
cat -n src/components/common/ImageCard.tsx
echo
echo "== imageSrc / coverImage usage =="
rg -n --context 2 "coverImage\\.url|imageSrc|ImageCard|<Image " src/components src/app src/lib src
echo
echo "== next.config.* =="
cat -n next.config.ts next.config.mjs next.config.js 2>/dev/null || true
echo
echo "== media/url references near payload config =="
rg -n --context 2 "images:|upload|url:|s3|cloudinary|remotePatterns|domains" src/payload.config.ts srcRepository: kir-dev/ehk
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/payload.config.ts (storage/plugins area) =="
sed -n '1,180p' src/payload.config.ts | cat -n
echo
echo "== media collection =="
fd -H -t f 'media*' src --exec sh -c 'echo "--- {}"; sed -n "1,220p" "{}" | cat -n' \;
echo
echo "== withPayload / image config docs in repo =="
rg -n --context 2 "remotePatterns|domains|images\\s*:|withPayload\\(" .Repository: kir-dev/ehk
Length of output: 5564
Whitelist the media origin for next/image
coverImage.url comes from Payload’s S3-backed media collection, but next.config.ts doesn’t define any images.remotePatterns or images.domains. If a dormitory card resolves to a remote S3 URL, this <Image> will fail at runtime. Add the media host to images.remotePatterns (or render these sources unoptimized).
🤖 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 `@src/components/common/ImageCard.tsx` around lines 30 - 38, The ImageCard
component uses next/image with a remote media URL from the Payload S3-backed
collection, but the app config does not whitelist that host. Update the Next.js
image configuration in next.config.ts to add the media origin via
images.remotePatterns or images.domains, and make sure the Image usage in
ImageCard still renders remote coverImage.url sources correctly.
2 participants
…tion
Summary by CodeRabbit
New Features
Bug Fixes