-
-
Notifications
You must be signed in to change notification settings - Fork 36
feat(views): add pdf template for outgoing speakers schedule #4622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(views): add pdf template for outgoing speakers schedule #4622
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…schedule_Max-Makaluk
WalkthroughAdds outgoing speakers schedule export: new OutgoingSpeakersScheduleType, a scheduleOutgoingSpeakers service builder, UI toggles to export weekend meeting and/or outgoing speakers, updated export orchestration, a React‑PDF template with supporting components and icon, plus a minor page-header border-radius tweak. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Weekend Export UI
participant Hook as useWeekendExport
participant Svc as schedules service
participant Tmpl as PDF Templates
User->>UI: Click "Export"
UI->>Hook: handleExportSchedules(weeks, options)
Hook->>Hook: validate inputs / set processing
alt Weekend Meeting checked
Hook->>Svc: build weekend meeting schedule
Svc-->>Hook: weekend schedule data
Hook->>Tmpl: render WeekendMeeting PDF
end
alt Outgoing Speakers checked
Hook->>Svc: scheduleOutgoingSpeakers(weeks)
Svc-->>Hook: OutgoingSpeakersScheduleType
Hook->>Tmpl: render TemplateOutgoingSpeakersSchedule
end
Hook-->>UI: complete (or error)
UI-->>User: Download(s) or error toast
sequenceDiagram
autonumber
participant Svc as scheduleOutgoingSpeakers
participant Store as App Store (talks/songs/lang)
participant Util as i18n + name utils
Svc->>Store: read outgoing_talks, publicTalks, songs, language
loop each outgoing talk
Svc->>Util: personGetDisplayName(speaker)
Svc->>Util: getTranslation(talk title, lang)
Svc->>Util: getTranslation(song title, lang)
Svc->>Util: generateMonthShortNames(lang)
Svc->>Svc: format date -> formattedDate
Svc->>Svc: push speak entry to result.speak
end
Svc-->>Caller: { speak: [...] } (OutgoingSpeakersScheduleType)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
🧹 Nitpick comments (2)
src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleSpeakBox.tsx (1)
61-61: Use strict inequality operator.For consistency with JavaScript best practices, use
!==instead of!=.- {index != data.speak.length - 1 && ( + {index !== data.speak.length - 1 && (src/features/meetings/weekend_export/useWeekendExport.tsx (1)
75-84: Simplify data building logic.The current approach builds an array of schedules and then merges them. This can be simplified to build the merged result directly.
- const outgoingSpeakersData: OutgoingSpeakersScheduleType[] = []; - - for (const schedule of weeksList) { - const data = scheduleOutgoingSpeakers(schedule); - outgoingSpeakersData.push(data); - } - - const mergedSchedule: OutgoingSpeakersScheduleType = { - speak: outgoingSpeakersData.flatMap((item) => item.speak), - }; + const mergedSchedule: OutgoingSpeakersScheduleType = { + speak: weeksList.flatMap((schedule) => + scheduleOutgoingSpeakers(schedule).speak + ), + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
src/locales/ceb-PH/forms-templates.jsonis excluded by!**/*.jsonsrc/locales/en/forms-templates.jsonis excluded by!**/*.jsonsrc/locales/en/meetings.jsonis excluded by!**/*.json
📒 Files selected for processing (14)
src/definition/schedules.ts(1 hunks)src/features/meetings/weekend_export/index.tsx(3 hunks)src/features/meetings/weekend_export/useWeekendExport.tsx(5 hunks)src/services/app/schedules.ts(4 hunks)src/views/components/icons/IconOutgoingSpeakers.tsx(1 hunks)src/views/components/page_header/index.tsx(1 hunks)src/views/index.ts(1 hunks)src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleBrotherBox.tsx(1 hunks)src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleContainer.tsx(1 hunks)src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleDateBox.tsx(1 hunks)src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleSpeakBox.tsx(1 hunks)src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleTalkBox.tsx(1 hunks)src/views/meetings/weekend/outgoing_speakers_schedule/index.tsx(1 hunks)src/views/meetings/weekend/outgoing_speakers_schedule/index.types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
src/views/components/page_header/index.tsx (1)
src/utils/common.ts (1)
getCSSPropertyValue(261-265)
src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleDateBox.tsx (2)
src/views/meetings/weekend/outgoing_speakers_schedule/index.types.ts (1)
OSScheduleDateBoxProps(17-20)src/utils/common.ts (1)
getCSSPropertyValue(261-265)
src/views/meetings/weekend/outgoing_speakers_schedule/index.types.ts (1)
src/definition/schedules.ts (1)
OutgoingSpeakersScheduleType(310-318)
src/features/meetings/weekend_export/useWeekendExport.tsx (2)
src/definition/schedules.ts (3)
SchedWeekType(77-126)WeekendMeetingDataType(279-308)OutgoingSpeakersScheduleType(310-318)src/services/app/schedules.ts (2)
schedulesWeekendData(2797-3068)scheduleOutgoingSpeakers(3070-3137)
src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleSpeakBox.tsx (1)
src/views/meetings/weekend/outgoing_speakers_schedule/index.types.ts (1)
OSScheduleSpeakBoxProps(13-15)
src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleBrotherBox.tsx (1)
src/views/meetings/weekend/outgoing_speakers_schedule/index.types.ts (1)
OSScheduleBrotherBoxProps(22-24)
src/views/meetings/weekend/outgoing_speakers_schedule/index.tsx (2)
src/views/meetings/weekend/outgoing_speakers_schedule/index.types.ts (1)
TemplateOutgoingSpeakersProps(3-7)src/utils/common.ts (1)
getCSSPropertyValue(261-265)
src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleTalkBox.tsx (2)
src/views/meetings/weekend/outgoing_speakers_schedule/index.types.ts (1)
OSScheduleTalkBoxProps(26-31)src/utils/common.ts (1)
getCSSPropertyValue(261-265)
src/views/components/icons/IconOutgoingSpeakers.tsx (1)
src/views/components/icons/index.types.ts (1)
IconProps(1-4)
src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleContainer.tsx (3)
src/views/meetings/weekend/outgoing_speakers_schedule/index.types.ts (1)
OSScheduleContainerProps(9-11)src/definition/schedules.ts (1)
OutgoingSpeakersScheduleType(310-318)src/utils/common.ts (1)
getCSSPropertyValue(261-265)
src/services/app/schedules.ts (7)
src/definition/schedules.ts (2)
SchedWeekType(77-126)OutgoingSpeakersScheduleType(310-318)src/states/public_talks.ts (1)
publicTalksState(5-5)src/states/settings.ts (3)
fullnameOptionState(86-95)displayNameMeetingsEnableState(186-199)JWLangState(201-211)src/states/persons.ts (1)
personsByViewState(95-111)src/states/songs.ts (1)
songsLocaleState(7-17)src/services/i18n/translation.ts (2)
generateMonthShortNames(72-89)getTranslation(3-19)src/utils/common.ts (1)
personGetDisplayName(184-204)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (7)
src/views/components/page_header/index.tsx (1)
23-26: Ensuredocumentis available for CSS property retrievalThe explicit corner radii are clear, but
getCSSPropertyValueaccessesdocument.documentElementat runtime. If any PDF rendering happens server-side (Node.js), this will throw. Confirm all PDF generation runs in a browser environment; otherwise extract those CSS variables into constants and use them instead ofgetCSSPropertyValue.src/definition/schedules.ts (1)
310-318: LGTM!The new
OutgoingSpeakersScheduleTypeis well-structured and clearly defines the data shape for the outgoing speakers schedule feature.src/views/index.ts (1)
8-8: LGTM!The export follows the established pattern for template exports in this file.
src/features/meetings/weekend_export/index.tsx (1)
54-71: LGTM!The checkbox UI for selecting export options is well-structured and follows the existing design patterns.
src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleBrotherBox.tsx (1)
1-75: LGTM!The component properly handles soft hyphen insertion for better text wrapping in PDF rendering, and the layout structure is clean.
src/features/meetings/weekend_export/useWeekendExport.tsx (1)
102-134: Verify handling when no export type is selected.If both
exportWeekendMeetingScheduleIsCheckedandexportOutgoingSpeakersScheduleIsCheckedare false, the function will complete successfully without generating any PDFs. This might be confusing UX.Consider adding validation:
const handleExportSchedules = async () => { if (startWeek.length === 0 || endWeek.length === 0) return; if (!exportWeekendMeetingScheduleIsChecked && !exportOutgoingSpeakersScheduleIsChecked) { displaySnackNotification({ header: getMessageByCode('error_app_generic-title'), message: 'Please select at least one schedule type to export', severity: 'warning', }); return; } // ... rest of function }Or disable the export button in the UI when both are unchecked.
src/views/meetings/weekend/outgoing_speakers_schedule/index.types.ts (1)
1-31: LGTM!The type definitions are clean, well-structured, and properly typed. They provide good type safety for the outgoing speakers schedule components.
src/views/meetings/weekend/outgoing_speakers_schedule/index.tsx
Outdated
Show resolved
Hide resolved
src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleContainer.tsx
Outdated
Show resolved
Hide resolved
src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleDateBox.tsx
Show resolved
Hide resolved
src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleDateBox.tsx
Show resolved
Hide resolved
src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleSpeakBox.tsx
Outdated
Show resolved
Hide resolved
src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleTalkBox.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleSpeakBox.tsx (1)
32-32: Fix key construction to use primitive value.The key still uses
speak.date(an object) which coerces to"[object Object]", defeating the purpose of a unique key. While you addedindex, the resulting key will be malformed (e.g.,"0_John_[object Object]").Apply this fix to use a primitive value:
- <Fragment key={`${index}_${speak.speaker}_${speak.date}`}> + <Fragment key={`${index}_${speak.speaker}_${speak.date.formatted}`}>src/features/meetings/weekend_export/useWeekendExport.tsx (1)
97-97: Filename pattern fix confirmed.Uses dash between weeks:
OS_${firstWeek}-${lastWeek}.pdf. This resolves the prior underscore inconsistency.
🧹 Nitpick comments (8)
src/views/components/icons/IconOutgoingSpeakers.tsx (1)
6-6: Consider simplifying the viewBox template literal.The viewBox uses a template literal with hardcoded values:
`0 0 ${32} ${32}`. Since 32 is not dynamic, this can be simplified to"0 0 32 32"for better readability.Apply this diff:
- <Svg width={size} height={size} viewBox={`0 0 ${32} ${32}`}> + <Svg width={size} height={size} viewBox="0 0 32 32">src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleSpeakBox.tsx (1)
61-61: Prefer strict equality.Use
!==instead of!=for type-safe comparison.- {index != data.speak.length - 1 && ( + {index !== data.speak.length - 1 && (src/features/meetings/weekend_export/useWeekendExport.tsx (6)
82-84: Chronologically sort merged outgoing schedule.Ensure consistent ordering across weeks by sorting by date.
Apply this diff:
- const mergedSchedule: OutgoingSpeakersScheduleType = { - speak: outgoingSpeakersData.flatMap((item) => item.speak), - }; + const mergedSchedule: OutgoingSpeakersScheduleType = { + speak: outgoingSpeakersData + .flatMap((item) => item.speak) + .sort( + (a, b) => a.date.date.getTime() - b.date.date.getTime() + ), + };
128-135: Sort weeks before exporting for stable filenames and output.Filter preserves source order; sort by
weekOfto ensurefirstWeek/lastWeekand PDF content are consistent.Apply this diff:
- if (exportWeekendMeetingScheduleIsChecked) { - await exportWeekendMeetingSchedule(weeksList); - } + const weeksListSorted = [...weeksList].sort((a, b) => + a.weekOf.localeCompare(b.weekOf) + ); + if (exportWeekendMeetingScheduleIsChecked) { + await exportWeekendMeetingSchedule(weeksListSorted); + } @@ - if (exportOutgoingSpeakersScheduleIsChecked) { - await exportOutgoingSpeakersSchedule(weeksList); - } + if (exportOutgoingSpeakersScheduleIsChecked) { + await exportOutgoingSpeakersSchedule(weeksListSorted); + }
108-111: String date comparison depends on lexicographic order — verify format.Ensure
weekOf,startWeek,endWeekare normalized (e.g., YYYY-MM-DD or YYYY/MM/DD zero‑padded) so>=/<=on strings is correct. Otherwise, parse to Date and compare timestamps.
102-104: Optional: guard when nothing is selected to export.Avoid a silent no‑op if both toggles are off.
Apply this diff:
- const handleExportSchedules = async () => { + const handleExportSchedules = async () => { if (startWeek.length === 0 || endWeek.length === 0) return; + if ( + !exportWeekendMeetingScheduleIsChecked && + !exportOutgoingSpeakersScheduleIsChecked + ) { + return; + }
138-149: Type‑safe error handling in catch.
catchvariable isunknownin TS; guard before accessing.message.Apply this diff:
- } catch (error) { - console.error(error); + } catch (error: unknown) { + console.error(error); @@ - displaySnackNotification({ + displaySnackNotification({ header: getMessageByCode('error_app_generic-title'), - message: getMessageByCode(error.message), + message: getMessageByCode( + error && typeof error === 'object' && 'message' in error + ? (error as Error).message + : 'error_app_generic-desc' + ), severity: 'error', });
58-60: Wider compatibility (optional): avoid.at()/.replaceAll().If targeting older browsers, prefer indexed access and regex replace.
Example:
const firstWeek = meetingData[0].weekOf.replace(/\//g, ''); const lastWeek = meetingData[meetingData.length - 1].weekOf.replace(/\//g, '');const firstWeek = weeksList[0].weekOf.replace(/\//g, ''); const lastWeek = weeksList[weeksList.length - 1].weekOf.replace(/\//g, '');Also applies to: 86-88
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/features/meetings/weekend_export/useWeekendExport.tsx(5 hunks)src/services/app/schedules.ts(4 hunks)src/views/components/icons/IconOutgoingSpeakers.tsx(1 hunks)src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleContainer.tsx(1 hunks)src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleSpeakBox.tsx(1 hunks)src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleTalkBox.tsx(1 hunks)src/views/meetings/weekend/outgoing_speakers_schedule/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/services/app/schedules.ts
- src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleTalkBox.tsx
- src/views/meetings/weekend/outgoing_speakers_schedule/index.tsx
- src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleContainer.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/views/components/icons/IconOutgoingSpeakers.tsx (1)
src/views/components/icons/index.types.ts (1)
IconProps(1-4)
src/features/meetings/weekend_export/useWeekendExport.tsx (2)
src/definition/schedules.ts (3)
SchedWeekType(77-126)WeekendMeetingDataType(279-308)OutgoingSpeakersScheduleType(310-318)src/services/app/schedules.ts (2)
schedulesWeekendData(2797-3068)scheduleOutgoingSpeakers(3070-3137)
src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleSpeakBox.tsx (1)
src/views/meetings/weekend/outgoing_speakers_schedule/index.types.ts (1)
OSScheduleSpeakBoxProps(13-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (6)
src/views/components/icons/IconOutgoingSpeakers.tsx (2)
4-4: Previous typo issue has been resolved.The component naming issue flagged in the previous review has been fixed. The component is now correctly named
IconOutgoingSpeakersat both the declaration and export, matching the filename.Also applies to: 29-29
1-29: LGTM! Clean icon component implementation.The component follows React best practices and correctly uses
@react-pdf/renderercomponents for PDF generation. The SVG structure is well-formed with:
- Proper prop destructuring with sensible defaults
- Consistent color application across all paths
- Correct usage of fillRule where needed
- Type-safe implementation using IconProps
src/features/meetings/weekend_export/useWeekendExport.tsx (4)
9-17: Imports and types wiring look correct.New types and services are properly imported and scoped.
19-22: Template imports look correct.Matches usage below; no concerns.
33-41: Toggles state: sensible defaults.Both exports enabled by default makes sense for UX.
152-166: Toggles + returned API look clean.Nice, straightforward handlers and exposure for the UI.
src/views/meetings/weekend/outgoing_speakers_schedule/OSScheduleSpeakBox.tsx
Show resolved
Hide resolved
…schedule_Max-Makaluk
|
| }; | ||
|
|
||
| export type OutgoingSpeakersScheduleType = { | ||
| speak: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FussuChalice: should this type be just an object, and later can be used as an array? Is it really necessary to have the nested key in it?
| const groupByDate = ( | ||
| speakers: OutgoingSpeakersScheduleType | ||
| ): OutgoingSpeakersScheduleType[] => { | ||
| const groups = speakers.speak.reduce<Record<number, typeof speakers.speak>>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FussuChalice: should the data sent here already formatted to be in an ascending order? Why we need to group it by date?
|
@FussuChalice: please check also the logic to get the meeting date. I suppose the existing function in |



Description
Fixes # (issue)
Type of change
Checklist: