139 implement redesigned student clubs - #179
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughChangesAdds a Payload-backed StudentClubs collection with migration and seeded legacy data. The page now fetches and renders localized student-club cards, including contacts, galleries, activities, and join prompts. Loading UI and bilingual page title/subtitle content were added. Student clubs
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Page
participant CMS
participant StudentClubCard
participant OrganizationCard
Page->>CMS: fetch ordered student clubs
CMS-->>Page: return localized club documents
Page->>StudentClubCard: provide club and locale
StudentClubCard->>OrganizationCard: provide mapped content, links, gallery, and join prompt
OrganizationCard-->>Page: render student-club card
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/collections/StudentClubs.ts (1)
101-112: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGenerated admin labels read oddly (
website URL,facebook URL).
name.replace("Url", " URL")leaves the first letter lowercase and untranslated, unlike the other Hungarian labels in this collection. Consider an explicit label map.♻️ Suggested explicit labels
- ...[ - "websiteUrl", - "facebookUrl", - "instagramUrl", - "linkedinUrl", - "youtubeUrl", - ].map((name) => ({ - name, - label: name.replace("Url", " URL"), + ...( + [ + ["websiteUrl", "Weboldal URL"], + ["facebookUrl", "Facebook URL"], + ["instagramUrl", "Instagram URL"], + ["linkedinUrl", "LinkedIn URL"], + ["youtubeUrl", "YouTube URL"], + ] as const + ).map(([name, label]) => ({ + name, + label, type: "text" as const, validate: validateOptionalUrl, })),🤖 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/StudentClubs.ts` around lines 101 - 112, Replace the generated labels in the URL field mapping with an explicit label map for websiteUrl, facebookUrl, instagramUrl, linkedinUrl, and youtubeUrl, using the collection’s established Hungarian wording and capitalization. Keep the existing field names, text types, and validateOptionalUrl validation unchanged.src/app/(app)/[lang]/kozelet/ontevekenykorok/page.tsx (1)
1-1: 🧹 Nitpick | 🔵 TrivialConsider ISR instead of
force-dynamicfor this mostly-static content.
force-dynamicre-fetches the dictionary and all student clubs on every request, bypassing Next.js's route cache entirely. Since club listings change infrequently, arevalidatevalue (ISR) would reduce CMS load and improve TTFB while still keeping content reasonably fresh.🤖 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]/kozelet/ontevekenykorok/page.tsx at line 1, Replace the force-dynamic route configuration with ISR by defining an appropriate revalidate interval for the page. Keep the dictionary and student-club data loading unchanged, allowing Next.js to cache the mostly static content while periodically refreshing it.src/migrations/20260726_210000_student_clubs_collection.ts (1)
163-167: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPositional pairing of hu/en organizations is fragile.
english = enOrganizations[order] ?? organizationpairs translations by array index rather than by theidfield both objects carry. If the two dictionaries ever diverge in order or length, this silently seeds the wrong (or Hungarian) text intopresentation_en, with no error. Matching byidwould be more robust against future dictionary edits.♻️ Suggested fix
- for (const [order, organization] of huOrganizations.entries()) { - const english = enOrganizations[order] ?? organization; + const enById = new Map(enOrganizations.map((org) => [org.id, org])); + for (const [order, organization] of huOrganizations.entries()) { + const english = enById.get(organization.id) ?? organization;🤖 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/20260726_210000_student_clubs_collection.ts` around lines 163 - 167, Update the organization pairing in the loop over huOrganizations to match each Hungarian organization with the English organization sharing its id, rather than using the array position. Preserve the existing fallback to the Hungarian organization when no matching English entry exists.src/app/(app)/[lang]/kozelet/ontevekenykorok/StudentClubCard.tsx (1)
20-23: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
||fallback treats an intentionally emptyjoinTextas missing.If an editor clears
joinText_en(empty string) to suppress the join prompt in one locale,||falls back tojoinText_huinstead of respecting the empty value.??distinguishesnull/undefinedfrom an intentional empty string.♻️ Suggested fix
const joinPrompt = locale === "en" - ? club.joinText_en || club.joinText_hu - : club.joinText_hu || club.joinText_en; + ? club.joinText_en ?? club.joinText_hu + : club.joinText_hu ?? club.joinText_en;🤖 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]/kozelet/ontevekenykorok/StudentClubCard.tsx around lines 20 - 23, Update the joinPrompt selection in StudentClubCard to use nullish fallback semantics instead of ||, so an intentionally empty joinText_en or joinText_hu is preserved while only null or undefined falls back to the other locale.
🤖 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]/kozelet/ontevekenykorok/studentClubCard.utils.ts:
- Around line 37-48: Update getGalleryImages to explicitly ensure image is
non-null before accessing image.url, while preserving the existing fallback to
club.name for missing alt text. Remove the now-unused Media import and eliminate
the redundant cast after narrowing the image union.
In `@src/migrations/20260726_210000_student_clubs_collection.ts`:
- Around line 96-101: Align the student_clubs_gallery.image_id nullability with
its foreign-key delete behavior in the migration: either allow image_id to be
nullable so ON DELETE SET NULL succeeds, or retain NOT NULL and change the
constraint to cascade-delete the gallery row. Apply the same correction to all
corresponding gallery table definitions, including the additional occurrence.
- Around line 163-219: Update the student-clubs migration loop using the
organization images data to create the referenced Media documents before
creating each student club, then populate that club’s gallery field with the
resulting media references. Ensure every migrated organization’s legacy gallery
is preserved and remains compatible with StudentClubCard’s club.gallery
rendering.
---
Nitpick comments:
In `@src/app/`(app)/[lang]/kozelet/ontevekenykorok/page.tsx:
- Line 1: Replace the force-dynamic route configuration with ISR by defining an
appropriate revalidate interval for the page. Keep the dictionary and
student-club data loading unchanged, allowing Next.js to cache the mostly static
content while periodically refreshing it.
In `@src/app/`(app)/[lang]/kozelet/ontevekenykorok/StudentClubCard.tsx:
- Around line 20-23: Update the joinPrompt selection in StudentClubCard to use
nullish fallback semantics instead of ||, so an intentionally empty joinText_en
or joinText_hu is preserved while only null or undefined falls back to the other
locale.
In `@src/collections/StudentClubs.ts`:
- Around line 101-112: Replace the generated labels in the URL field mapping
with an explicit label map for websiteUrl, facebookUrl, instagramUrl,
linkedinUrl, and youtubeUrl, using the collection’s established Hungarian
wording and capitalization. Keep the existing field names, text types, and
validateOptionalUrl validation unchanged.
In `@src/migrations/20260726_210000_student_clubs_collection.ts`:
- Around line 163-167: Update the organization pairing in the loop over
huOrganizations to match each Hungarian organization with the English
organization sharing its id, rather than using the array position. Preserve the
existing fallback to the Hungarian organization when no matching English entry
exists.
🪄 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 Plus
Run ID: d2785d97-4e13-485c-bd8b-76b574239e16
📒 Files selected for processing (16)
src/app/(app)/[lang]/kozelet/ontevekenykorok/StudentClubCard.tsxsrc/app/(app)/[lang]/kozelet/ontevekenykorok/loading.tsxsrc/app/(app)/[lang]/kozelet/ontevekenykorok/page.tsxsrc/app/(app)/[lang]/kozelet/ontevekenykorok/studentClubCard.utils.tssrc/collections/StudentClubs.tssrc/components/common/OrganizationCard.tsxsrc/components/common/organization-card/sections/JoinCta.tsxsrc/components/common/organization-card/types.tssrc/dictionaries/en/ontevekeny_korok.jsonsrc/dictionaries/hu/ontevekeny_korok.jsonsrc/lib/payload-cms.tssrc/migrations/20260726_210000_student_clubs_collection.jsonsrc/migrations/20260726_210000_student_clubs_collection.tssrc/migrations/index.tssrc/payload-types.tssrc/payload.config.ts
| export function getGalleryImages(club: StudentClub) { | ||
| return ( | ||
| club.gallery | ||
| ?.map(({ image }) => | ||
| typeof image === "object" && image.url | ||
| ? { src: image.url, alt: (image as Media).alt || club.name } | ||
| : null, | ||
| ) | ||
| .filter((image): image is { src: string; alt: string } => Boolean(image)) ?? | ||
| [] | ||
| ); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard against a null gallery relation.
typeof null === "object", so a dangling/unresolved media relation makes image.url throw during render. The as Media cast is also redundant once the union is narrowed.
🛡️ Proposed fix
export function getGalleryImages(club: StudentClub) {
return (
club.gallery
?.map(({ image }) =>
- typeof image === "object" && image.url
- ? { src: image.url, alt: (image as Media).alt || club.name }
+ image && typeof image === "object" && image.url
+ ? { src: image.url, alt: image.alt || club.name }
: null,
)Note: the Media import becomes unused after this change.
📝 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 function getGalleryImages(club: StudentClub) { | |
| return ( | |
| club.gallery | |
| ?.map(({ image }) => | |
| typeof image === "object" && image.url | |
| ? { src: image.url, alt: (image as Media).alt || club.name } | |
| : null, | |
| ) | |
| .filter((image): image is { src: string; alt: string } => Boolean(image)) ?? | |
| [] | |
| ); | |
| } | |
| export function getGalleryImages(club: StudentClub) { | |
| return ( | |
| club.gallery | |
| ?.map(({ image }) => | |
| image && typeof image === "object" && image.url | |
| ? { src: image.url, alt: image.alt || club.name } | |
| : null, | |
| ) | |
| .filter((image): image is { src: string; alt: string } => Boolean(image)) ?? | |
| [] | |
| ); | |
| } |
🤖 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]/kozelet/ontevekenykorok/studentClubCard.utils.ts around
lines 37 - 48, Update getGalleryImages to explicitly ensure image is non-null
before accessing image.url, while preserving the existing fallback to club.name
for missing alt text. Remove the now-unused Media import and eliminate the
redundant cast after narrowing the image union.
| CREATE TABLE "student_clubs_gallery" ( | ||
| "_order" integer NOT NULL, | ||
| "_parent_id" integer NOT NULL, | ||
| "id" varchar PRIMARY KEY NOT NULL, | ||
| "image_id" integer NOT NULL | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
image_id is NOT NULL but its FK uses ON DELETE set null — contradiction.
student_clubs_gallery.image_id is declared NOT NULL, yet the FK constraint sets ON DELETE set null. Deleting a Media row referenced by any gallery entry will fail with a not-null constraint violation instead of gracefully nulling the reference, breaking media deletion for any admin who tries to remove a photo used in a club gallery.
🐛 Suggested fix — pick one
- "image_id" integer NOT NULL
+ "image_id" integeror, if the relation must remain required, cascade-delete the gallery row instead:
ALTER TABLE "student_clubs_gallery"
ADD CONSTRAINT "student_clubs_gallery_image_id_media_id_fk"
FOREIGN KEY ("image_id") REFERENCES "public"."media"("id")
- ON DELETE set null ON UPDATE no action;
+ ON DELETE cascade ON UPDATE no action;Also applies to: 130-133
🤖 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/20260726_210000_student_clubs_collection.ts` around lines 96 -
101, Align the student_clubs_gallery.image_id nullability with its foreign-key
delete behavior in the migration: either allow image_id to be nullable so ON
DELETE SET NULL succeeds, or retain NOT NULL and change the constraint to
cascade-delete the gallery row. Apply the same correction to all corresponding
gallery table definitions, including the additional occurrence.
| const huOrganizations = getOrganizations("hu"); | ||
| const enOrganizations = getOrganizations("en"); | ||
|
|
||
| for (const [order, organization] of huOrganizations.entries()) { | ||
| const english = enOrganizations[order] ?? organization; | ||
| const contacts = getContacts(organization); | ||
| const isAerospace = organization.id === "bme_aerospace"; | ||
|
|
||
| await payload.create({ | ||
| collection: "student-clubs", | ||
| data: { | ||
| name: organization.title, | ||
| presentation_hu: richTextDocument(organization.description), | ||
| presentation_en: richTextDocument(english.description), | ||
| contacts, | ||
| joinText_hu: isAerospace | ||
| ? "Érdekel az űrkutatás és a rakétatechnika?" | ||
| : "Szeretnél csatlakozni ehhez a közösséghez?", | ||
| joinText_en: isAerospace | ||
| ? "Interested in space research and rocket technology?" | ||
| : "Would you like to join this community?", | ||
| joinUrl: | ||
| contacts.websiteUrl ?? | ||
| contacts.facebookUrl ?? | ||
| contacts.instagramUrl, | ||
| order, | ||
| ...(isAerospace && { | ||
| activities: [ | ||
| { text_hu: "Rakétaépítés", text_en: "Rocket building" }, | ||
| { | ||
| text_hu: "Tudományos kísérletek", | ||
| text_en: "Scientific experiments", | ||
| }, | ||
| { | ||
| text_hu: "Fedélzeti elektronika fejlesztése", | ||
| text_en: "Onboard electronics development", | ||
| }, | ||
| { | ||
| text_hu: "Antennarendszerek tervezése", | ||
| text_en: "Antenna system design", | ||
| }, | ||
| { | ||
| text_hu: "Aerodinamikai szabályozás", | ||
| text_en: "Aerodynamic control", | ||
| }, | ||
| ], | ||
| targetAudience_hu: richTextDocument([ | ||
| "Minden BME hallgatónak, akit érdekel az űrkutatás, az űrtechnológia és a rakétatechnika — különösen azoknak, akik nem csak elméleti tudást, hanem valódi fejlesztési tapasztalatot szeretnének szerezni.", | ||
| ]), | ||
| targetAudience_en: richTextDocument([ | ||
| "For every BME student interested in space research, space technology and rocket engineering — especially those who want real development experience in addition to theoretical knowledge.", | ||
| ]), | ||
| }), | ||
| }, | ||
| req, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate migration files =="
git ls-files | rg 'src/migrations/.*student_clubs|student clups|student-clubs|LegacyOrganization|StudentClub|Media|Gallery|gallery' || true
echo
echo "== target migration relevant content =="
target="$(git ls-files | rg 'src/migrations/20260726_210000_student_clubs_collection\.ts$' | head -n1 || true)"
if [ -n "${target:-}" ]; then
wc -l "$target"
sed -n '1,260p' "$target" | cat -n
fi
echo
echo "== search for images field / Media gallery usage =="
rg -n "images|Media|gallery|galleryImages|getGalleryImages|StudentClub|student-clubs|student clubs|LegacyOrganization|legacy.*organization" -S .Repository: kir-dev/ehk
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant migration references =="
git ls-files 'src/migrations/*student_clubs*' 'src/migrations/*gallery*clubs*' | while read -r f; do
echo "--- $f ---"
wc -l "$f"
rg -n "images|gallery|payload\.create|payload\.find|media|Media|student-clubs|LegacyOrganization|ontevekeny_korok" "$f" || true
done
echo
echo "== student club dictionaries image snippets =="
python3 - <<'PY'
import json, pathlib
for p in ["src/dictionaries/hu/ontevekeny_korok.json", "src/dictionaries/en/ontevekeny_korok.json"]:
data = json.loads(pathlib.Path(p).read_text())
print(p)
for kor in data.get("ontevekeny_korok", {}).get("korok", []):
img = kor.get("images")
print(f" {kor['id']}: images={img}")
PY
echo
echo "== student club gallery/helper code =="
sed -n '1,140p' src/collections/StudentClubs.ts | cat -n
echo
sed -n '1,70p' 'src/app/(app)/[lang]/kozelet/ontevekenykorok/StudentClubCard.tsx' | cat -n
echo
sed -n '1,70p' src/app/\(app\)/\[lang\]/kozelet/ontevekenykorok/studentClubCard.utils.ts | cat -n
echo
echo "== any payload.find with student-clubs gallery references near migrations/helpers =="
rg -n "collection:\s*[\"']student-clubs[\"']|collection:\s*'student-clubs'|\bgallery\b|getGalleryImages|student clubs gallery|student-clubs_gallery|student_clubs_gallery" src -SRepository: kir-dev/ehk
Length of output: 28546
Preserve legacy organization galleries in the student-clubs migration.
Both dictionaries include images entries, but the migration only creates student_clubs and an empty student_clubs_gallery table — it never creates the referenced Media docs or fills gallery. Since StudentClubCard renders from club.gallery, every migrated club will launch with an empty gallery unless a separate media backfill is added.
🤖 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/20260726_210000_student_clubs_collection.ts` around lines 163
- 219, Update the student-clubs migration loop using the organization images
data to create the referenced Media documents before creating each student club,
then populate that club’s gallery field with the resulting media references.
Ensure every migrated organization’s legacy gallery is preserved and remains
compatible with StudentClubCard’s club.gallery rendering.
Summary by CodeRabbit
New Features
Bug Fixes