✨ Finish event creation and editing flow - #952
Conversation
|
Follow-up on #945: instructions for making the calendar reusable Functionally this PR already covers the ask (fetch/create/edit/delete events, month grid, upcoming/past lists). What's still needed is a refactor so The page (
1. Consolidate the data logic into a hook
2. Split into componentsTwo new pieces, both currently inlined in (a) Reusable
The (b) "Create an event" CTA — extract the 3.
|
nadavosa
left a comment
There was a problem hiding this comment.
Reviewed the diff and checked it out locally. tsc --noEmit and yarn lint both pass clean on this branch (only the same 4 pre-existing, unrelated hook warnings you already called out). Note GitHub hasn't actually run CI on this PR yet, it's a fork PR and no check runs exist for the head commit yet (probably needs a maintainer to approve the workflow run), so "CI passes" isn't confirmed from GitHub's side yet, just from my local run.
Found a few real issues worth addressing before merge:
1. Event type is hardcoded to WORKSHOP for every event. toPayload() in CreateEvent.tsx always sets type: EventN4DType.WORKSHOP, but the SDK enum has two values (PARTY, WORKSHOP) and there's no UI to choose. Every event a coordinator creates through this flow, including a purely social one, gets tagged as a workshop. Was this intentional (e.g. "party" isn't used yet), or should there be a type selector in the form?
2. Events only get a translation in the coordinator's current UI language. toPayload() submits a single-element translations array using i18n.language. Since the public site is bilingual and translations is a required array (per ApiEventN4DCreate), an event created by a German-speaking coordinator has no English translation record at all, and vice versa. Non-matching-locale visitors to the public event page would presumably see nothing (or a broken fallback) for that event. This seems like a real gap for a bilingual site rather than an edge case.
3. Multi-day events aren't reflected on the calendar past their start day. eventDays (for the dot markers) and clickDay (click-to-scroll-to-event) in CalendarContent.tsx both key only on event.date, never event.dateEnd. Since the date/time step explicitly supports a different end date (with end > start validation), a coordinator can create a multi-day event, but the calendar only marks its first day. Clicking the event's other days (including its own end day) shows no marker and offers "create event" there instead, since nothing flags that day as occupied.
4. Multi-day event card display reads as a same-day range. EventCard's range() helper builds the date from event.date but appends endTime from event.dateEnd regardless of whether they're the same calendar day, e.g. a Friday 18:00 → Saturday 09:00 event renders as "Friday, ... · 18:00–09:00", which reads like an invalid same-day range rather than a two-day span.
5. Minor: /dashboard/calendar/[id]/edit/page.tsx does Number(id), if the id in the URL isn't numeric this becomes NaN, which is falsy, so CreateEvent silently falls into "create new event" mode instead of showing an error for a malformed edit URL. Low severity, just flagging.
None of these are typos or crashes, the flow works for the straightforward same-day, single-language case shown in the screenshots, but 1-3 look like they'd affect real usage once coordinators create multi-day or non-English-only events.
|
@arturasmckwcz @nadavosa Thank you both for reviewing the PR and for the detailed feedback. I’ve pushed commit For Arturas’s refactor request:
For Nadav’s findings:
One question remains about translations: should coordinators enter both English and German title/description, or should the public event pages rely on the backend fallback when only one translation exists? Updated calendar
Event type selector
Please let me know if I should make any further changes to PR #952. |
nadavosa
left a comment
There was a problem hiding this comment.
Re-reviewed after the latest push (daec62e). Pulled the branch, tsc --noEmit and yarn lint both pass, and CI is green now too.
All previously flagged issues are genuinely fixed, verified in code, not just claimed:
- Event type —
StepTitle.tsxnow has a real<select>with bothWORKSHOPandPARTY(both translation keys present in en/de), wired throughformData.typeintotoPayload(). Confirmed. - Multi-day calendar marking —
utils/calendar.ts'seventDateKeys()now expands the fulldate→dateEndrange, andCalendarGridmarks every day in that range (with a count badge, nice touch). Confirmed. - Multi-day range display —
eventDateRange()now branches ondateKey(start) === dateKey(end), showing "start · time – end date · time" when they differ instead of mixing the two into a same-day-looking range. Confirmed. - Malformed edit URL —
[id]/edit/page.tsxnow validatesNumber.isInteger(eventId) && eventId > 0and callsnotFound()otherwise, instead of silently falling into create-mode. Confirmed.
On the open translation question: checked the be DTO (dto-event.ts) — resolveTranslation() already falls back to whichever translation exists when the requested language's isn't there (event.eventTranslation?.[0]), per the be#903 comment in that file. So relying on the backend fallback is fine, coordinators don't need to enter both languages for this to work; the event just won't be localized for the other language until someone adds that translation. No further fe change needed for that.
On the refactor (Arturas's ask): CalendarContent.tsx is now a clean 60-line orchestrator (hook + CTA + Calendar), and the day-click/highlight logic in useCalendar.ts + groupEventsByDate correctly handles the "highlight the whole day's group, not just one event" case, including for multi-day events (clicking a non-start day of a multi-day event still resolves and highlights the correct group). Nice work, this looks solid to me.
No further blockers from my side.
arturasmckwcz
left a comment
There was a problem hiding this comment.
Re-review findings (automated).
| type: formData.type, | ||
| linkRSVP: formData.registrationLink, | ||
| address: `${formData.street} ${formData.houseNumber}, ${formData.postcode} Berlin`, | ||
| active: true, |
There was a problem hiding this comment.
toPayload() hardcodes active: true on every submit (CONFIRMED)
BE's PATCH /event/:id treats active as a genuine partial-update field (omitted = unchanged, via TypeORM's undefined-skips-column behavior in write-event.ts). EventCard.tsx even renders a Published/Draft badge from event.active, implying drafts are real. But toPayload() is shared by create and edit and always sends active: true with no UI control — a coordinator who opens a draft event just to fix a typo and saves will unintentionally publish it, with no way to un-publish through this flow.
| linkRSVP: formData.registrationLink, | ||
| address: `${formData.street} ${formData.houseNumber}, ${formData.postcode} Berlin`, | ||
| active: true, | ||
| translations: [ |
There was a problem hiding this comment.
Edit form omits fields BE treats as explicit-clear (CONFIRMED)
toPayload()'s translations entry omits subTitle, locationComment, additionalTitle, additionalInfo, outro and followUpText. be/src/server/utils/data/write-event.ts's translationFields() sets each of these to t.field ?? null on save — deliberate per its own comment ("leaving these as undefined would silently keep the old value instead of clearing it"). editingEvent does carry subTitle/locationComment/additionalTitle/additionalInfo, but the form never reads or re-sends them, so any event with these fields set gets wiped to null the moment it's edited through this form.
| active: true, | ||
| translations: [ | ||
| { | ||
| language: i18n.language === Lang.DE ? Lang.DE : Lang.EN, |
There was a problem hiding this comment.
Edit under mismatched UI language duplicates translation row (CONFIRMED)
useEvents() calls GET /event with no language param, so BE resolves under default Lang.DE; dto-event.ts's resolveTranslation() falls back to eventTranslation[0] (any language) when no DE row exists. If a German-UI coordinator edits an event whose only translation is English, the form displays that English content, but toPayload() tags the submission language: Lang.DE. BE's per-language upsert then inserts a new DE row containing the English text, leaving the original English translation untouched and duplicated under the wrong language.
| return false; | ||
| }; | ||
|
|
||
| if (eventId && isLoading) |
There was a problem hiding this comment.
Blank page on missing/failed event edit, no way back (CONFIRMED)
Both the "still loading" branch (here, line 150) and the "event not found" branch (line 156) render an identical empty <PageContent /> with no message and no way back. Navigating to /calendar/<id>/edit for an id that never resolves (stale link, event deleted by someone else, or a fetch failure) leaves the user staring at a permanently blank page — no error text, no retry, no link back to the calendar.
| <Page> | ||
| <PageHeading> | ||
| <Heading2>{t("dashboard.calendar.calendarTitle")}</Heading2> | ||
| <CreateEventCta onCreate={() => calendar.createEvent()} /> |
There was a problem hiding this comment.
Create-event CTA lost date prefill from selected day (CONFIRMED)
<CreateEventCta onCreate={() => calendar.createEvent()} /> always calls createEvent with no date argument. selectDate() only auto-navigates when the clicked day has no events; a day that already has events just gets highlighted/scrolled. A coordinator who selects such a day then clicks the top button, expecting the new event prefilled with that date, silently loses the prefill — a regression vs. the old handleCreateEvent this replaced.
| } | ||
|
|
||
| function parseAddress(address = "") { | ||
| const match = address.match(/^(.*)\s+(\S+),\s*(\d{5})/); |
There was a problem hiding this comment.
Non-matching address format blocks edit flow (PLAUSIBLE)
parseAddress()'s regex only matches a strict "Street Number, PPPPP…" shape. BE's address column has no format constraint beyond minLength: 1. An address that doesn't match the pattern falls back to {street: address, houseNumber: '', postcode: ''}; step 2's isNextEnabled requires houseNumber/postcode non-empty, so the coordinator gets stuck on step 2 with no explanation.
| </Meta> | ||
| <Meta> | ||
| <LinkIcon size={18} /> | ||
| <Registration href={event.linkRSVP} target="_blank" rel="noopener noreferrer"> |
There was a problem hiding this comment.
No fallback for empty/invalid linkRSVP href (PLAUSIBLE)
The registration link renders <a href={event.linkRSVP}> with no fallback. BE's schema only requires linkRSVP to be a string (no format/minLength check). An event created outside this new FE flow with an empty linkRSVP makes "Open registration link" just reload the current page.
| const changeMonth = (offset: number) => { | ||
| setMonthDate((current) => { | ||
| const next = new Date(current.getFullYear(), current.getMonth() + offset, 1); | ||
| setShowPast(next < new Date(today.getFullYear(), today.getMonth(), 1)); |
There was a problem hiding this comment.
Impure setState side effect inside functional updater (PLAUSIBLE)
setShowPast is invoked as a side effect inside the setMonthDate functional updater, which React requires to be pure. React can invoke a functional updater more than once (Strict Mode double-invoke, or future concurrent-rendering paths); each extra invocation re-fires setShowPast as an unintended side effect. Idempotent today, but fragile and couples unrelated state through hidden call order.
| return useGetQuery<ApiEventN4DGetList[]>({ queryKey: EVENT_QUERY_KEY, apiPath: apiPathEvent }); | ||
| } | ||
|
|
||
| export function useEvent(id?: number) { |
There was a problem hiding this comment.
useEvent fetches full list instead of single resource (CONFIRMED)
useGetVolunteer/useGetOpportunity both fetch one resource by id. useEvent instead pulls the full list via useEvents() (ApiEventN4DGetList shape, which lacks hostName/time/outro/followUpText/followUpLink/locationLink) and finds by id client-side, so the edit form can never read or preserve those fields for an event that has them — compounding the translation-field data loss on save (see the comment on line 115).
| }); | ||
| } | ||
|
|
||
| export function useDeleteEvent() { |
There was a problem hiding this comment.
useDeleteEvent hand-rolls axios instead of useMutationQuery (CONFIRMED)
useDeleteOpportunity/useDeleteAgent both go through useMutationQuery({apiPath, method: 'delete', ...}) with no direct axios import. useDeleteEvent imports axios directly instead, violating fe/CLAUDE.md's "use these hooks, don't call axios directly" and shared-rules.md's "reuse before you create," for no functional reason since useMutationQuery already supports this case.
|
@nadavosa @arturasmckwcz Thanks for the re-review. I addressed the FE-actionable findings in commit
|
|
@nadavosa @arturasmckwcz Follow-up fix pushed in b32b9ea:
Verified with yarn typecheck, yarn lint (four pre-existing hook warnings), yarn build, and git diff --check. One backend-dependent limitation remains: when the title or description is edited, the frontend cannot determine whether GET /event returned the requested language or a fallback. Fully resolving this requires the API to return the resolved language and complete event/translation details, ideally through GET /event/:id. |
|
Fixed and pushed in commit 5549f17. The event-type dropdown now:
Type-checking, linting, and the production build pass. |


Description
Finishes the dashboard event-management flow so coordinators can create, view, edit, and delete persisted events.
The calendar now displays real event data and provides a clearer overview of upcoming and past events. The existing three-step creation design is preserved while adding the missing event fields and backend integration.
Related Issues
Closes #945
Changes
need4deed-sdkpackage.Screenshots / Demos
Before
After
Events dashboard — German
Events dashboard and event flow — English
Testing
yarn typecheck— passedyarn lint— passed with four pre-existing, unrelated React Hook warningsgit diff --check— passedChecklist