Conversation
* Interface web admin * ajout .env.example * Meilleure gestion des rôles, ajout bouton vérifier un utilisateur, fix quelques comportements relous * mobile "friendly" * fix users verified * refacto et ajout recherche * Ajout menu RU * Menu mieux * fix viewport * ajout bassine * Biome fix * oups * rabbit
Introduces a modal in the admin UI for managing club responsibles, allowing admins to add or remove responsibles for a club. Adds supporting React hooks and API methods, updates backend with a new handler and route for removing responsibles, and updates dependencies and dev server port.
Introduces support for custom warning and confirmation messages on reservation items. Adds database columns, admin UI modal for editing messages, API endpoints for fetching and updating messages, and updates backend models and queries to handle these fields. Enables club owners and admins to manage reservation dialog messages per item.
Introduces a full reservation management interface in the admin panel, including a reservation tree page, modals for category and item creation/editing, and message customization. Adds React components, API types, React hooks, and Go backend handlers for CRUD operations on reservation categories and items. Updates the sidebar to include a link to the new reservations section and extends the API and routes to support the new features.
Moved Club, Event, and User type imports from './api' to './types' in relevant files for better type organization and separation. This change improves code clarity and maintains a cleaner type structure.
Replaces Plus icons with FolderPlus and PackagePlus for category and item actions to improve clarity. Adds display of item descriptions in the reservation list and updates folder toggling for club items to use icons instead of a spacer.
Improves the construction of SQL INSERT queries in CreateCategory and CreateItem by dynamically building column lists and parameter placeholders. This change ensures correct parameter numbering and simplifies handling of optional fields, reducing potential errors and improving maintainability.
Introduces a migration that resets the sequences for reservation_category and reservation_element tables to prevent duplicate key errors by aligning them with the current maximum IDs.
Introduces push notifications for new events and event reminders, with localized messages based on user language. Adds a migration for tracking sent notifications to prevent duplicates, updates the event handler to send notifications on event creation, and implements an event scheduler to send reminders 1 hour before events. Updates i18n files for notification translations and extends the notification service with new user/token retrieval methods.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a complete admin app (Next.js frontend, components, hooks, Zustand stores), a centralized Axios client with React Query hooks, backend admin handlers/routes, notification service and scheduler, DB migrations (notifications, reservations, allergens), and project configuration files. Changes
Sequence Diagram(s)sequenceDiagram
participant AdminUI as Admin UI
participant API as Backend Admin API
participant DB as Database
participant Notif as Notification Service
AdminUI->>API: POST /admin/events (create event)
API->>DB: INSERT event
DB-->>API: returns event id
API->>API: spawn goroutine to process notifications
API->>DB: query users + tokens grouped by language
DB-->>API: users with tokens
API->>Notif: send localized push per language group
Notif-->>API: send results
API->>DB: INSERT event_notification_sent (mark sent)
sequenceDiagram
participant AdminUI as Admin UI
participant API as Backend Admin API
participant DB as Database
AdminUI->>API: GET /admin/reservation/tree
API->>DB: fetch categories and items (include messages)
DB-->>API: nested reservation tree
API-->>AdminUI: reservation tree
AdminUI->>API: PATCH /admin/reservation-items/:id (update messages)
API->>DB: UPDATE reservation_element (warning_message, confirmation_message)
DB-->>API: update result
API-->>AdminUI: success
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code graph analysis (2)handlers/restaurant/service/menu_service.go (1)
handlers/restaurant/repository/menu_repository.go (2)
🔇 Additional comments (9)
✏️ Tip: You can disable this entire section by setting 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.
Pull request overview
This PR introduces a comprehensive admin panel for the Transat backend application along with several backend improvements. The changes include a complete Next.js-based admin dashboard, event notification system, multi-device notification token support, and enhanced reservation management.
Changes:
- Migrated notification token storage to support multiple devices per user via
user_notification_tokenstable - Added automated event reminder notifications (1 hour before events) via a new scheduler
- Created a full-featured admin panel with user, event, club, menu, and reservation management
- Enhanced club responsible management to support multiple owners per club
- Added custom warning/confirmation messages for reservation items
Reviewed changes
Copilot reviewed 87 out of 91 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| services/notification.go | Updated to use new user_notification_tokens table, added methods for event/club interest queries |
| scheduler/event_scheduler.go | New scheduler for sending event reminder notifications 1h before start |
| scheduler/scheduler.go | Integrated EventScheduler into main scheduler |
| handlers/user/user_handlers.go | Changed notification token storage to new table, updated GraduationYear to pointer |
| handlers/club/club_handler.go | Enhanced to support multiple club responsibles, added RemoveClubRespo endpoint |
| handlers/admin/admin_handlers.go | New comprehensive admin handlers for user, event, club, menu, reservation, and bassine management |
| models/*.go | Updated models for nullable GraduationYear, reservation messages, notification tokens |
| middlewares/adminAuth.go | New middleware for admin authentication checks |
| routes/*.go | Added admin routes and new club/statistics endpoints |
| main.go | Updated CORS config for admin panel, integrated schedulers |
| db/migrations/*.sql | New migrations for user_notification_tokens, reservation messages, event notifications |
| admin/* | Complete Next.js admin panel with components, hooks, stores, and pages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Build query dynamically with correct parameter numbering | ||
| columns := []string{"name", "id_clubs"} | ||
| placeholders := []string{"$1", "$2"} | ||
| args := []interface{}{category.Name, *category.IDClubParent} |
There was a problem hiding this comment.
Potential nil pointer dereference when dereferencing category.IDClubParent. The field is a pointer and should be checked for nil before dereferencing. If it's guaranteed to be non-nil at this point, add validation earlier in the function.
| // Build query dynamically with correct parameter numbering | ||
| columns := []string{"name", "slot", "id_clubs"} | ||
| placeholders := []string{"$1", "$2", "$3"} | ||
| args := []interface{}{item.Name, item.Slot, *item.IDClubParent} |
There was a problem hiding this comment.
Potential nil pointer dereference when dereferencing item.IDClubParent. Similar to the category creation, this should be validated before use or the pointer should be checked for nil.
| AllowMethods: "GET, POST, PUT, DELETE, OPTIONS", | ||
| AllowHeaders: "Origin, Content-Type, Accept, Authorization, X-Requested-With", | ||
| AllowMethods: "GET, POST, PUT, PATCH, DELETE, OPTIONS", | ||
| AllowOrigins: "http://localhost:3001", // pour le pannel admin en dev |
There was a problem hiding this comment.
Corrected spelling of 'pannel' to 'panel' in the comment.
| // Create table if it doesn't exist (should be done via migration, but this is a safety check) | ||
| createTableQuery := ` | ||
| CREATE TABLE IF NOT EXISTS event_notification_sent ( |
There was a problem hiding this comment.
Creating tables in application code is not recommended. While the comment acknowledges this should be done via migration, the CREATE TABLE IF NOT EXISTS logic should be removed and rely solely on migrations. This could cause issues in production environments where the application user may not have CREATE TABLE permissions.
| const findItemInTree = ( | ||
| tree: any[], | ||
| itemId: number, | ||
| ): { warning_message?: string; confirmation_message?: string } | null => { |
There was a problem hiding this comment.
Using any type defeats the purpose of TypeScript's type safety. Define proper types for the tree structure or use the existing ReservationTreeItem type from the API module.
| } catch (error: any) { | ||
| alert(error?.response?.data?.error || "Erreur lors de l'enregistrement"); |
There was a problem hiding this comment.
Using alert() for error messages is not ideal for user experience. Consider using the toast notification system that's already being used elsewhere in the application (react-hot-toast).
| } catch (error: any) { | ||
| alert(error?.response?.data?.error || "Erreur lors de l'enregistrement"); |
There was a problem hiding this comment.
Using alert() for error messages is not ideal. Consider using the toast notification system that's already being used elsewhere in the application (react-hot-toast).
| const deleteItem = useDeleteReservationItem(); | ||
|
|
||
| const [expandedNodes, setExpandedNodes] = useState<Set<string>>(new Set()); | ||
| const [selectedCategory, setSelectedCategory] = useState<ReservationTreeItem | null>(null); |
There was a problem hiding this comment.
Unused variable selectedCategory.
| const [selectedCategory, setSelectedCategory] = useState<ReservationTreeItem | null>(null); |
| const deleteItem = useDeleteReservationItem(); | ||
|
|
||
| const [expandedNodes, setExpandedNodes] = useState<Set<string>>(new Set()); | ||
| const [selectedCategory, setSelectedCategory] = useState<ReservationTreeItem | null>(null); |
There was a problem hiding this comment.
Unused variable setSelectedCategory.
| const [selectedCategory, setSelectedCategory] = useState<ReservationTreeItem | null>(null); | |
| const [selectedCategory] = useState<ReservationTreeItem | null>(null); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
handlers/club/club_handler.go (1)
754-757: Remove debug logging of raw request body.Logging raw request bodies can expose sensitive data (e.g., user emails) in production logs, which may raise privacy/compliance concerns. This appears to be debug code that should be removed before merging.
Suggested fix
- // Debug: Log the raw body content - bodyBytes := c.Body() - utils.LogLineKeyValue(utils.LevelInfo, "Raw Body", string(bodyBytes)) - utils.LogLineKeyValue(utils.LevelInfo, "Content-Type", c.Get("Content-Type")) - var req models.AddRespoRequest if err := c.BodyParser(&req); err != nil { utils.LogMessage(utils.LevelError, "Failed to parse request body") utils.LogLineKeyValue(utils.LevelError, "Error", err) - utils.LogLineKeyValue(utils.LevelError, "Raw Body", string(bodyBytes)) - utils.LogLineKeyValue(utils.LevelError, "Content-Type", c.Get("Content-Type")) utils.LogFooter()handlers/auth/auth_handlers.go (2)
148-154: Transaction handling after duplicate user has potential issues.After rolling back the transaction on duplicate key error, a new transaction is started but the error from
h.DB.Begin()at line 151 is only logged viafmt.Println(inconsistent with other logging) and execution continues regardless. If this transaction start fails, subsequent operations usingtxwill fail unexpectedly.Consider either returning early on duplicate user or properly handling the transaction error.
Suggested improvement
commitOrRollback(tx, err) // Rollback - tx, err = h.DB.Begin() - if err != nil { - fmt.Println("Failed to begin database transaction") - } - + // User already exists, we'll just update their verification code + tx, err = h.DB.Begin() + if err != nil { + utils.LogMessage(utils.LevelError, "Failed to begin new transaction after duplicate user") + utils.LogLineKeyValue(utils.LevelError, "Error", err) + utils.LogFooter() + return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"error": "Database transaction error"}) + } alreadyExists = true
179-184: Same transaction error handling issue.This is the same pattern as lines 148-154 - if the new transaction fails to start, execution continues with an invalid
txhandle.services/notification.go (3)
70-122: Prepared statement not closed on all error paths.The statement
stmtcreated at line 79 is only closed at line 117-121, but ifstmt.Query()fails (line 86-91), the statement is not closed before returning. This causes a resource leak.Proposed fix using defer
stmt, err := ns.db.Prepare(query) if err != nil { log.Println("║ 💥 Failed to prepare statement: ", err) log.Println("╚=========================================╝") return nil, err } +defer stmt.Close() rows, err := stmt.Query(pq.Array(groups)) if err != nil { log.Println("║ 💥 Failed to query groups: ", err) log.Println("╚=========================================╝") return nil, err } - -defer func(rows *sql.Rows) { - err := rows.Close() - if err != nil { - log.Println("║ 💥 Failed to close rows: ", err) - log.Println("╚=========================================╝") - } -}(rows) +defer rows.Close() // ... rest of code ... -if err := stmt.Close(); err != nil { - log.Println("║ 💥 Failed to close statement: ", err) - log.Println("╚=========================================╝") - return nil, err -}
39-67: Missingrows.Err()check after iteration in the first query block.The email-based query (lines 39-68) iterates over rows but does not check
rows.Err()after the loop. This could miss errors that occurred during iteration.Proposed fix
targets = append(targets, target) } + if err := rows.Err(); err != nil { + log.Println("║ 💥 Failed to iterate rows: ", err) + log.Println("╚=========================================╝") + return nil, err + } }
283-291: HTTP client lacks timeout configuration.Creating
&http.Client{}without a timeout means requests to the Expo API could hang indefinitely if the server is unresponsive, potentially exhausting resources.Proposed fix
-client := &http.Client{} +client := &http.Client{ + Timeout: 30 * time.Second, +}You'll also need to add
"time"to the imports.
🤖 Fix all issues with AI agents
In @admin/app/login/page.tsx:
- Around line 9-18: The useSearchParams() call inside LoginPage requires a React
Suspense boundary in Next.js 15; either wrap the component that calls
useSearchParams in a <Suspense> or move the search-param logic into a small
client subcomponent. Fix by extracting the search param read into a new client
component (e.g., SearchParamsReader) that uses useSearchParams to read the
"verified" param and calls a prop callback (e.g., onVerified) or returns the
verified UI, then render <Suspense fallback={null}><SearchParamsReader
onVerified={setShowVerifiedMessage} /></Suspense> from LoginPage (or
alternatively wrap the parent that renders LoginPage in <Suspense>), ensuring
you remove direct useSearchParams() from LoginPage and keep state setters like
setShowVerifiedMessage intact.
In @admin/components/Combobox.tsx:
- Around line 37-38: The generated buttonId using Math.random in Combobox.tsx is
unstable across renders; replace that line to produce a stable id (e.g., use
React's useId() or create a stable ref/memo) so the htmlFor/id association and
SSR hydration remain consistent; update the variable named buttonId (and any
places referencing it) to derive its value from useId() (or useRef/useMemo)
inside the Combobox component and remove the Math.random usage.
In @admin/components/ReservationItemsModal.tsx:
- Around line 41-52: The handleSave function currently calls
updateMessages.mutateAsync without error handling and always resets local state;
wrap the mutateAsync call in a try/catch and use toast from react-hot-toast to
notify the user: call toast.success(...) on successful mutation and only then
call setEditingItem(null), setWarningMessage(""), and
setConfirmationMessage(""); in the catch block call toast.error(...) with the
error message so failures are surfaced; reference handleSave and
updateMessages.mutateAsync when implementing the try/catch and toast calls.
In @admin/lib/performance.ts:
- Around line 74-99: The hook lacks unmount cleanup and treats options as
unstable; fix by stabilizing the callback with a ref (create callbackRef in
useIntersectionObserver, set callbackRef.current = callback in an effect) and
use callbackRef.current when constructing the IntersectionObserver inside
observe, add a useEffect that returns a cleanup which disconnects
observer.current on unmount (and when options change), and address the unstable
options by deriving a stable dependency (e.g., memoize or serialize options with
useMemo/JSON.stringify and use that stable value in your observe/effect
dependencies) so observe/disconnect no longer recreate the observer every
render.
- Around line 55-71: The function useStableCallback mutates refs during render
which violates React purity; change the ref-update logic to run inside a
useEffect (add useEffect to imports) so depsRef.current and callbackRef.current
are updated after render, and in that effect compare both array lengths and
elements (e.g., check length mismatch or any dep !== depsRef.current[i]) to
detect changes including shorter arrays before assigning the new dependencies
and callback; keep returning the stable useCallback wrapper (empty deps) so the
callback identity remains stable while avoiding render-side effects.
In @admin/lib/router.tsx:
- Around line 7-12: Router.tsx duplicates the lazy-loaded page components
already defined in admin/components/LazyComponents.tsx (DashboardPage,
UsersPage, EventsPage, ClubsPage, LoginPage); consolidate by moving the lazy
import + loading/error wrapper into a single module (e.g., keep
LazyComponents.tsx as the source of truth) and have admin/lib/router.tsx
re-export or import those named components instead of redefining them so
maintenance and behavior stay consistent.
In @admin/next.config.ts:
- Around line 7-18: The images.remotePatterns configuration is too permissive
(uses hostname: "**") which allows Next.js to proxy images from any host;
replace the wildcard entries in images.remotePatterns with an explicit allowlist
of trusted hostnames (e.g., add objects with protocol "https"/"http" and
hostname set to each trusted domain) and remove the wildcard hostname entries so
that only those trusted domains are permitted by the images configuration;
update the images object in next.config.ts (the remotePatterns array) to contain
only those specific hostnames and protocols, or alternatively implement
server-side URL validation before passing URLs to Next.js Image if dynamic
sources are required.
In @handlers/reservation/repository/reservation_repository.go:
- Around line 160-164: The code dereferences *item.IDClubParent when building
args (columns/placeholders/args/paramNum) which can panic; in the CreateItem
function add an explicit nil check for item.IDClubParent before using
*item.IDClubParent and handle it (e.g., return a descriptive error from
CreateItem such as "missing IDClubParent" or validate earlier) rather than
dereferencing; update any logic that builds columns/placeholders/args (variables
columns, placeholders, args, paramNum) to only use *item.IDClubParent after the
nil check so the function cannot panic.
- Around line 117-128: The code dereferences *category.IDClubParent when
building args (columns/placeholders/args/paramNum) which can panic if
IDClubParent is nil; update the logic in the block that initializes
columns/placeholders/args so you first check if category.IDClubParent != nil
before appending and using *category.IDClubParent, and if that field is required
return a validation error (or if optional, append a nil value or omit the column
entirely) so that functions using columns/placeholders/args (and variables
paramNum, category.Name) never perform a direct dereference of a potentially nil
pointer.
In @scheduler/event_scheduler.go:
- Around line 208-224: Remove the defensive CREATE TABLE logic from
markNotificationSent on EventScheduler: the inline createTableQuery and the
es.db.Exec call that attempts to create event_notification_sent should be
deleted so schema is managed only via migrations (migration
00022_event_notification_sent.sql); ensure markNotificationSent only performs
the insert/update logic for recording a sent notification and does not attempt
any DDL, and remove or adjust any related error logging in that function that
referenced the table creation attempt.
- Around line 58-64: In EventScheduler.checkAndSendEventNotifications replace
time.Now() with time.Now().UTC() when computing
oneHourFromNow/windowStart/windowEnd (variables oneHourFromNow, windowStart,
windowEnd) so Go-side times are in UTC, and update the DB side to use UTC-aware
comparisons (either ensure events.start_date is stored in UTC or migrate
start_date to TIMESTAMP WITH TIME ZONE and use timezone-aware NOW() in queries)
to avoid server vs DB timezone mismatches.
🟡 Minor comments (13)
handlers/statistics/statistics_handlers.go-87-117 (1)
87-117: Errors are logged but not surfaced—partial data with zero values may mislead consumers.When individual queries fail, the code logs the error but continues, returning zero values for failed metrics. This could be intentional (best-effort dashboard), but the caller cannot distinguish between "zero users" and "query failed."
Consider either:
- Return an error response if any critical query fails, or
- Include per-field error indicators in the response, or
- Document this best-effort behavior for consumers.
handlers/statistics/statistics_handlers.go-134-145 (1)
134-145: Missingrows.Err()check after iteration.After iterating through
rows.Next(), you should checkrows.Err()to catch any errors that occurred during iteration (e.g., network issues mid-stream).🔧 Proposed fix
if err := rows.Scan(&date, &count, &cumulativeCount); err == nil { stats.UserGrowth = append(stats.UserGrowth, map[string]interface{}{ "date": date, "count": count, "cumulativeCount": cumulativeCount, }) } } + if err := rows.Err(); err != nil { + utils.LogMessage(utils.LevelError, "Error during user growth rows iteration") + utils.LogLineKeyValue(utils.LevelError, "Error", err) + } }admin/lib/schemas.ts-21-38 (1)
21-38: Graduation year validation mismatch with backend.The frontend schema allows
currentYear + 10, but the backend (handlers/user/user_handlers.goline 187) validates againstcurrentYear + 5. This inconsistency will cause valid frontend submissions to fail on the backend.🔧 Align with backend validation
.refine( (val) => { if (val === undefined) return true; const currentYear = new Date().getFullYear(); - return val >= 1900 && val <= currentYear + 10; + return val >= 1900 && val <= currentYear + 5; }, { message: "Année de diplôme invalide", }, ),admin/app/clubs/page.tsx-220-231 (1)
220-231: Potential issue:ClubOwnersModalreceives nullableclubId.
selectedClubIdcan benull, butClubOwnersModalis always rendered (not conditionally likeReservationItemsModalon line 232). If the modal component doesn't handlenullgracefully, this could cause issues.Consider applying the same conditional rendering pattern:
Suggested fix
- <ClubOwnersModal - isOpen={ownersModalOpen} - onClose={() => setOwnersModalOpen(false)} - clubId={selectedClubId} - clubName={selectedClubName} - /> + {selectedClubId && ( + <ClubOwnersModal + isOpen={ownersModalOpen} + onClose={() => setOwnersModalOpen(false)} + clubId={selectedClubId} + clubName={selectedClubName} + /> + )}admin/components/ErrorBoundary.tsx-20-30 (1)
20-30:retryCountpersists after successful recovery, which may cause unexpected page reloads.The
retryCountinstance property is never reset when children render successfully after a retry. If an error occurs, the user retries successfully, and then a different error occurs later,retryCountretains its previous value. This could cause an unexpected page reload if the user has already used retries on previous errors.🔧 Suggested fix - reset retryCount on successful render
render() { if (this.state.hasError) { return this.props.fallback || this.getErrorUI(); } + // Reset retry count on successful render + if (this.retryCount > 0) { + this.retryCount = 0; + } + return this.props.children; }admin/app/reservations/page.tsx-152-153 (1)
152-153: Non-null assertion onnode.idmay cause runtime issues.If
node.idis everundefined, the non-null assertion (!) will passundefinedtohandleDeleteCategory, which expects anumber.🔧 Add a guard or handle undefined case
<button - onClick={() => handleDeleteCategory(node.id!)} + onClick={() => node.id && handleDeleteCategory(node.id)} className="p-1 text-red-600 hover:bg-red-50 rounded" title="Supprimer" >admin/components/EventModal.tsx-98-102 (1)
98-102: Handle emptyid_clubto prevent NaN.If
formData.id_clubis an empty string,parseIntreturnsNaN, which could cause backend issues. Consider adding validation or a fallback.🐛 Suggested fix
try { + if (!formData.id_club) { + setError("Veuillez sélectionner un club"); + return; + } + const eventData = { ...formData, id_club: parseInt(formData.id_club, 10), };admin/components/CategoryModal.tsx-52-68 (1)
52-68: Non-null assertion oncategory.idmay cause runtime error.When
categoryexists butidisundefined(which is possible per theReservationTreeItemtype whereid?: number), the non-null assertion on line 55 will passundefinedto the API, potentially causing unexpected behavior.🛡️ Suggested defensive check
try { if (category) { + if (category.id === undefined) { + alert("Impossible de modifier une catégorie sans ID"); + return; + } await updateCategory.mutateAsync({ - id: category.id!, + id: category.id, data: { name: name.trim() }, }); } else {admin/lib/stores/appStore.ts-94-99 (1)
94-99: Potential race condition when rapidly reopening modals.The 150ms timeout to clear
editingUseris not cancelled if the modal is reopened before the timeout fires. This could result in the editing state being cleared while the modal is open with a new user.🔧 Consider storing timeout IDs to cancel on reopen
One approach is to store timeout IDs and clear them in the open actions:
+let userModalTimeout: ReturnType<typeof setTimeout> | null = null; openUserModal: (user) => { + if (userModalTimeout) { + clearTimeout(userModalTimeout); + userModalTimeout = null; + } set({ userModalOpen: true, editingUser: user || null, }); }, closeUserModal: () => { set({ userModalOpen: false }); - setTimeout(() => { + userModalTimeout = setTimeout(() => { set({ editingUser: null }); + userModalTimeout = null; }, 150); },The same pattern would apply to club and event modals.
admin/components/UserModal.tsx-53-80 (1)
53-80: Form state not reset when modal reopens in create mode.The
useEffectdepends only on[user]. If a user opens the modal in create mode, enters data, closes without saving, then reopens to create another user, the form retains the previous values becauseuserremainsnulland the effect doesn't re-run.🔧 Proposed fix
useEffect(() => { if (user) { setFormData({ email: user.email, // ... existing user fields }); } else { setFormData({ email: "", // ... reset all fields }); } setError(""); - }, [user]); + }, [user, isOpen]);scheduler/event_scheduler.go-37-40 (1)
37-40: Potential blocking send on stop channel.If
Stop()is called when the goroutine is not currently blocked on the select (e.g., duringcheckAndSendEventNotifications()), the send tostopChanwill block until the next select iteration. Consider using a buffered channel or a non-blocking send pattern.Proposed fix using buffered channel
func NewEventScheduler(db *sql.DB, notificationService *services.NotificationService) *EventScheduler { return &EventScheduler{ db: db, notificationService: notificationService, - stopChan: make(chan bool), + stopChan: make(chan bool, 1), } }admin/components/LazyComponents.tsx-31-43 (1)
31-43:withPageLazyLoadinglacks ErrorBoundary unlikerouter.tsx's version.The
withRouteLoadinginrouter.tsxwraps components with bothErrorBoundaryandSuspense, butwithPageLazyLoadinghere only usesSuspense. This inconsistency means errors in lazy-loaded pages might not be caught gracefully.Proposed fix to add ErrorBoundary
+import ErrorBoundary from "./ErrorBoundary"; + export function withPageLazyLoading<P extends object>( Component: React.LazyExoticComponent<React.ComponentType<P>>, loadingText?: string, ) { return function LazyPageWrapper(props: P) { return ( + <ErrorBoundary level="page"> <Suspense fallback={<FullPageLoading text={loadingText || "Chargement de la page..."} />}> <Component {...props} /> </Suspense> + </ErrorBoundary> ); }; }admin/lib/hooks.ts-281-290 (1)
281-290: Missing cache invalidation for user-specific history after score update.When
useUpdateBassineScoresucceeds, it invalidates["bassine-scores"]but not["bassine-history", userEmail]. If a user is viewing history while a score update occurs, the history view will show stale data until manually refreshed.Consider invalidating the affected user's history query as well.
🔧 Proposed fix
export const useUpdateBassineScore = () => { const queryClient = useQueryClient(); return useMutation({ mutationFn: bassineApi.updateScore, - onSuccess: () => { + onSuccess: (_data, variables) => { queryClient.invalidateQueries({ queryKey: ["bassine-scores"] }); + queryClient.invalidateQueries({ queryKey: ["bassine-history", variables.userEmail] }); }, }); };
| export default function LoginPage() { | ||
| const [email, setEmail] = useState(""); | ||
| const [password, setPassword] = useState(""); | ||
| const [error, setError] = useState(""); | ||
| const [loading, setLoading] = useState(false); | ||
| const [showVerifiedMessage, setShowVerifiedMessage] = useState(false); | ||
| const router = useRouter(); | ||
| const searchParams = useSearchParams(); | ||
| const emailId = useId(); | ||
| const passwordId = useId(); |
There was a problem hiding this comment.
Wrap useSearchParams usage in a Suspense boundary.
In Next.js 15, useSearchParams() requires the component to be wrapped in a <Suspense> boundary. Without it, the page will throw an error during static rendering. Since this component reads the verified query parameter, consider either:
- Wrapping the component in
<Suspense>in a parent, or - Splitting the search params logic into a separate client component wrapped with Suspense.
Suggested fix: Create a wrapper with Suspense
+"use client";
+
+import { Suspense } from "react";
+
+function LoginPageContent() {
+ // ... existing component code
+}
+
export default function LoginPage() {
- const [email, setEmail] = useState("");
- // ... rest of component
+ return (
+ <Suspense fallback={<div className="min-h-screen bg-gray-50 flex items-center justify-center">Loading...</div>}>
+ <LoginPageContent />
+ </Suspense>
+ );
}🤖 Prompt for AI Agents
In @admin/app/login/page.tsx around lines 9 - 18, The useSearchParams() call
inside LoginPage requires a React Suspense boundary in Next.js 15; either wrap
the component that calls useSearchParams in a <Suspense> or move the
search-param logic into a small client subcomponent. Fix by extracting the
search param read into a new client component (e.g., SearchParamsReader) that
uses useSearchParams to read the "verified" param and calls a prop callback
(e.g., onVerified) or returns the verified UI, then render <Suspense
fallback={null}><SearchParamsReader onVerified={setShowVerifiedMessage}
/></Suspense> from LoginPage (or alternatively wrap the parent that renders
LoginPage in <Suspense>), ensuring you remove direct useSearchParams() from
LoginPage and keep state setters like setShowVerifiedMessage intact.
| // Generate unique ID for accessibility | ||
| const buttonId = `combobox-${Math.random().toString(36).substr(2, 9)}`; |
There was a problem hiding this comment.
Unique ID generated on every render breaks accessibility.
Math.random() is called on every render, producing a new buttonId each time. This breaks the htmlFor/id association and can cause React hydration mismatches in SSR.
Use useId() (React 18+) or useMemo with a stable reference:
🐛 Recommended fix using useId
-import { useEffect, useRef, useState } from "react";
+import { useEffect, useId, useRef, useState } from "react";
// Inside the component:
- const buttonId = `combobox-${Math.random().toString(36).substr(2, 9)}`;
+ const buttonId = useId();📝 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.
| // Generate unique ID for accessibility | |
| const buttonId = `combobox-${Math.random().toString(36).substr(2, 9)}`; | |
| // Generate unique ID for accessibility | |
| const buttonId = useId(); |
🤖 Prompt for AI Agents
In @admin/components/Combobox.tsx around lines 37 - 38, The generated buttonId
using Math.random in Combobox.tsx is unstable across renders; replace that line
to produce a stable id (e.g., use React's useId() or create a stable ref/memo)
so the htmlFor/id association and SSR hydration remain consistent; update the
variable named buttonId (and any places referencing it) to derive its value from
useId() (or useRef/useMemo) inside the Combobox component and remove the
Math.random usage.
| const handleSave = async (itemId: number) => { | ||
| await updateMessages.mutateAsync({ | ||
| itemId, | ||
| messages: { | ||
| warning_message: warningMessage.trim() || null, | ||
| confirmation_message: confirmationMessage.trim() || null, | ||
| }, | ||
| }); | ||
| setEditingItem(null); | ||
| setWarningMessage(""); | ||
| setConfirmationMessage(""); | ||
| }; |
There was a problem hiding this comment.
Missing error handling in save operation.
The handleSave function uses mutateAsync without a try/catch block. If the mutation fails, the error is silently swallowed and the user receives no feedback about the failure. Additionally, the state reset on lines 49-51 won't execute on error.
🔧 Proposed fix
const handleSave = async (itemId: number) => {
+ try {
await updateMessages.mutateAsync({
itemId,
messages: {
warning_message: warningMessage.trim() || null,
confirmation_message: confirmationMessage.trim() || null,
},
});
setEditingItem(null);
setWarningMessage("");
setConfirmationMessage("");
+ } catch (error) {
+ // Error is already handled by react-query, but optionally show inline feedback
+ console.error("Failed to save messages:", error);
+ }
};Consider using toast from react-hot-toast (as used in UserModal.tsx) to provide user feedback on success/failure.
📝 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.
| const handleSave = async (itemId: number) => { | |
| await updateMessages.mutateAsync({ | |
| itemId, | |
| messages: { | |
| warning_message: warningMessage.trim() || null, | |
| confirmation_message: confirmationMessage.trim() || null, | |
| }, | |
| }); | |
| setEditingItem(null); | |
| setWarningMessage(""); | |
| setConfirmationMessage(""); | |
| }; | |
| const handleSave = async (itemId: number) => { | |
| try { | |
| await updateMessages.mutateAsync({ | |
| itemId, | |
| messages: { | |
| warning_message: warningMessage.trim() || null, | |
| confirmation_message: confirmationMessage.trim() || null, | |
| }, | |
| }); | |
| setEditingItem(null); | |
| setWarningMessage(""); | |
| setConfirmationMessage(""); | |
| } catch (error) { | |
| // Error is already handled by react-query, but optionally show inline feedback | |
| console.error("Failed to save messages:", error); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In @admin/components/ReservationItemsModal.tsx around lines 41 - 52, The
handleSave function currently calls updateMessages.mutateAsync without error
handling and always resets local state; wrap the mutateAsync call in a try/catch
and use toast from react-hot-toast to notify the user: call toast.success(...)
on successful mutation and only then call setEditingItem(null),
setWarningMessage(""), and setConfirmationMessage(""); in the catch block call
toast.error(...) with the error message so failures are surfaced; reference
handleSave and updateMessages.mutateAsync when implementing the try/catch and
toast calls.
| export function useStableCallback<T extends (...args: unknown[]) => unknown>( | ||
| callback: T, | ||
| dependencies: unknown[], | ||
| ): T { | ||
| const callbackRef = useRef<T>(callback); | ||
| const depsRef = useRef(dependencies); | ||
|
|
||
| // Update callback if dependencies changed | ||
| if (dependencies.some((dep, i) => dep !== depsRef.current[i])) { | ||
| callbackRef.current = callback; | ||
| depsRef.current = dependencies; | ||
| } | ||
|
|
||
| return useCallback((...args: Parameters<T>) => { | ||
| return callbackRef.current(...args); | ||
| }, []) as T; | ||
| } |
There was a problem hiding this comment.
Side effect during render violates React's purity rules.
Mutating refs based on dependency comparison during render (lines 63-66) is a side effect that breaks React's assumption that render is pure. This can cause issues with:
- StrictMode double-renders
- Concurrent rendering features
- Future React versions
Additionally, the dependency comparison doesn't handle array length changes (if new array is shorter, extra old deps aren't detected).
Recommended fix using useEffect for ref updates
export function useStableCallback<T extends (...args: unknown[]) => unknown>(
callback: T,
- dependencies: unknown[],
): T {
const callbackRef = useRef<T>(callback);
- const depsRef = useRef(dependencies);
- // Update callback if dependencies changed
- if (dependencies.some((dep, i) => dep !== depsRef.current[i])) {
- callbackRef.current = callback;
- depsRef.current = dependencies;
- }
+ // Update ref in effect to avoid side effects during render
+ useEffect(() => {
+ callbackRef.current = callback;
+ });
return useCallback((...args: Parameters<T>) => {
return callbackRef.current(...args);
}, []) as T;
}Note: You'll need to add useEffect to the imports. This pattern always keeps the ref up-to-date after each render while maintaining a stable callback identity.
🤖 Prompt for AI Agents
In @admin/lib/performance.ts around lines 55 - 71, The function
useStableCallback mutates refs during render which violates React purity; change
the ref-update logic to run inside a useEffect (add useEffect to imports) so
depsRef.current and callbackRef.current are updated after render, and in that
effect compare both array lengths and elements (e.g., check length mismatch or
any dep !== depsRef.current[i]) to detect changes including shorter arrays
before assigning the new dependencies and callback; keep returning the stable
useCallback wrapper (empty deps) so the callback identity remains stable while
avoiding render-side effects.
| export function useIntersectionObserver( | ||
| callback: (entries: IntersectionObserverEntry[]) => void, | ||
| options?: IntersectionObserverInit, | ||
| ) { | ||
| const observer = useRef<IntersectionObserver | null>(null); | ||
|
|
||
| const observe = useCallback( | ||
| (element: Element) => { | ||
| if (observer.current) { | ||
| observer.current.disconnect(); | ||
| } | ||
|
|
||
| observer.current = new IntersectionObserver(callback, options); | ||
| observer.current.observe(element); | ||
| }, | ||
| [callback, options], | ||
| ); | ||
|
|
||
| const disconnect = useCallback(() => { | ||
| if (observer.current) { | ||
| observer.current.disconnect(); | ||
| } | ||
| }, []); | ||
|
|
||
| return { observe, disconnect }; | ||
| } |
There was a problem hiding this comment.
Missing cleanup on unmount and unstable options dependency.
Two issues with this hook:
-
Missing unmount cleanup: If the component unmounts without calling
disconnect(), the observer continues running, potentially causing memory leaks or errors when the callback fires. -
Unstable
optionsdependency: Object reference comparison means if the caller doesn't memoizeoptions, every render creates a new observer.
Recommended fix with proper cleanup
export function useIntersectionObserver(
callback: (entries: IntersectionObserverEntry[]) => void,
options?: IntersectionObserverInit,
) {
const observer = useRef<IntersectionObserver | null>(null);
+ const callbackRef = useRef(callback);
+ callbackRef.current = callback;
const observe = useCallback(
(element: Element) => {
if (observer.current) {
observer.current.disconnect();
}
- observer.current = new IntersectionObserver(callback, options);
+ observer.current = new IntersectionObserver(
+ (entries) => callbackRef.current(entries),
+ options
+ );
observer.current.observe(element);
},
- [callback, options],
+ [options?.root, options?.rootMargin, options?.threshold],
);
const disconnect = useCallback(() => {
if (observer.current) {
observer.current.disconnect();
}
}, []);
+ // Cleanup on unmount
+ useEffect(() => {
+ return () => {
+ if (observer.current) {
+ observer.current.disconnect();
+ }
+ };
+ }, []);
return { observe, disconnect };
}This stabilizes the callback via ref and adds automatic cleanup on unmount. You'll need to add useEffect to imports.
🤖 Prompt for AI Agents
In @admin/lib/performance.ts around lines 74 - 99, The hook lacks unmount
cleanup and treats options as unstable; fix by stabilizing the callback with a
ref (create callbackRef in useIntersectionObserver, set callbackRef.current =
callback in an effect) and use callbackRef.current when constructing the
IntersectionObserver inside observe, add a useEffect that returns a cleanup
which disconnects observer.current on unmount (and when options change), and
address the unstable options by deriving a stable dependency (e.g., memoize or
serialize options with useMemo/JSON.stringify and use that stable value in your
observe/effect dependencies) so observe/disconnect no longer recreate the
observer every render.
| // Send notification to all users about the new event (with translations) | ||
| go func() { | ||
| users, err := h.notificationService.GetAllUsersWithLanguage() | ||
| if err != nil { | ||
| log.Printf("Failed to get all users with language for new event notification: %v", err) | ||
| return | ||
| } | ||
|
|
||
| if len(users) == 0 { | ||
| log.Println("No users found for new event notification") | ||
| return | ||
| } | ||
|
|
||
| // Group users by language | ||
| languageGroups := make(map[string][]models.NotificationTargetWithLanguage) | ||
| for _, user := range users { | ||
| if user.NotificationToken != "" { | ||
| langCode := user.LanguageCode | ||
| if langCode == "" { | ||
| langCode = "fr" // Default to French | ||
| } | ||
| languageGroups[langCode] = append(languageGroups[langCode], user) | ||
| } | ||
| } | ||
|
|
||
| totalSent := 0 | ||
| // Send notifications to each language group | ||
| for langCode, langUsers := range languageGroups { | ||
| localizer := appI18n.GetLocalizer(langCode) | ||
|
|
||
| title := localizer.MustLocalize(&i18n.LocalizeConfig{ | ||
| MessageID: "event_notification.new_event_title", | ||
| DefaultMessage: &i18n.Message{ | ||
| ID: "event_notification.new_event_title", | ||
| Other: "New event", | ||
| }, | ||
| }) | ||
|
|
||
| message := localizer.MustLocalize(&i18n.LocalizeConfig{ | ||
| MessageID: "event_notification.new_event_message", | ||
| TemplateData: map[string]interface{}{ | ||
| "EventName": req.Name, | ||
| "Location": req.Location, | ||
| }, | ||
| DefaultMessage: &i18n.Message{ | ||
| ID: "event_notification.new_event_message", | ||
| Other: "{{.EventName}} - {{.Location}}", | ||
| }, | ||
| }) | ||
|
|
||
| var tokens []string | ||
| for _, user := range langUsers { | ||
| tokens = append(tokens, user.NotificationToken) | ||
| } | ||
|
|
||
| payload := models.NotificationPayload{ | ||
| NotificationTokens: tokens, | ||
| Title: title, | ||
| Message: message, | ||
| Sound: "default", | ||
| ChannelID: "default", | ||
| Data: map[string]interface{}{ | ||
| "screen": "Events", | ||
| "eventId": eventID, | ||
| }, | ||
| } | ||
|
|
||
| if err := h.notificationService.SendPushNotification(payload); err != nil { | ||
| log.Printf("Failed to send new event notification to %s users: %v", langCode, err) | ||
| continue | ||
| } | ||
|
|
||
| log.Printf("Successfully sent new event notification to %d users in %s", len(tokens), langCode) | ||
| totalSent += len(tokens) | ||
| } | ||
|
|
||
| log.Printf("Successfully sent new event notification to %d users across %d languages", totalSent, len(languageGroups)) | ||
| }() |
There was a problem hiding this comment.
Goroutine lacks panic recovery and timeout control.
The notification goroutine could panic (e.g., if MustLocalize fails unexpectedly or if the notification service has an unhandled error). Additionally, there's no timeout or context, so the goroutine could run indefinitely if SendPushNotification hangs.
🔒️ Add panic recovery and timeout
// Send notification to all users about the new event (with translations)
go func() {
+ defer func() {
+ if r := recover(); r != nil {
+ log.Printf("Panic in event notification goroutine: %v", r)
+ }
+ }()
+
users, err := h.notificationService.GetAllUsersWithLanguage()
if err != nil {
log.Printf("Failed to get all users with language for new event notification: %v", err)
return
}
// ... rest of the code
}()Consider also adding a context with timeout for the notification sending operations.
📝 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.
| // Send notification to all users about the new event (with translations) | |
| go func() { | |
| users, err := h.notificationService.GetAllUsersWithLanguage() | |
| if err != nil { | |
| log.Printf("Failed to get all users with language for new event notification: %v", err) | |
| return | |
| } | |
| if len(users) == 0 { | |
| log.Println("No users found for new event notification") | |
| return | |
| } | |
| // Group users by language | |
| languageGroups := make(map[string][]models.NotificationTargetWithLanguage) | |
| for _, user := range users { | |
| if user.NotificationToken != "" { | |
| langCode := user.LanguageCode | |
| if langCode == "" { | |
| langCode = "fr" // Default to French | |
| } | |
| languageGroups[langCode] = append(languageGroups[langCode], user) | |
| } | |
| } | |
| totalSent := 0 | |
| // Send notifications to each language group | |
| for langCode, langUsers := range languageGroups { | |
| localizer := appI18n.GetLocalizer(langCode) | |
| title := localizer.MustLocalize(&i18n.LocalizeConfig{ | |
| MessageID: "event_notification.new_event_title", | |
| DefaultMessage: &i18n.Message{ | |
| ID: "event_notification.new_event_title", | |
| Other: "New event", | |
| }, | |
| }) | |
| message := localizer.MustLocalize(&i18n.LocalizeConfig{ | |
| MessageID: "event_notification.new_event_message", | |
| TemplateData: map[string]interface{}{ | |
| "EventName": req.Name, | |
| "Location": req.Location, | |
| }, | |
| DefaultMessage: &i18n.Message{ | |
| ID: "event_notification.new_event_message", | |
| Other: "{{.EventName}} - {{.Location}}", | |
| }, | |
| }) | |
| var tokens []string | |
| for _, user := range langUsers { | |
| tokens = append(tokens, user.NotificationToken) | |
| } | |
| payload := models.NotificationPayload{ | |
| NotificationTokens: tokens, | |
| Title: title, | |
| Message: message, | |
| Sound: "default", | |
| ChannelID: "default", | |
| Data: map[string]interface{}{ | |
| "screen": "Events", | |
| "eventId": eventID, | |
| }, | |
| } | |
| if err := h.notificationService.SendPushNotification(payload); err != nil { | |
| log.Printf("Failed to send new event notification to %s users: %v", langCode, err) | |
| continue | |
| } | |
| log.Printf("Successfully sent new event notification to %d users in %s", len(tokens), langCode) | |
| totalSent += len(tokens) | |
| } | |
| log.Printf("Successfully sent new event notification to %d users across %d languages", totalSent, len(languageGroups)) | |
| }() | |
| // Send notification to all users about the new event (with translations) | |
| go func() { | |
| defer func() { | |
| if r := recover(); r != nil { | |
| log.Printf("Panic in event notification goroutine: %v", r) | |
| } | |
| }() | |
| users, err := h.notificationService.GetAllUsersWithLanguage() | |
| if err != nil { | |
| log.Printf("Failed to get all users with language for new event notification: %v", err) | |
| return | |
| } | |
| if len(users) == 0 { | |
| log.Println("No users found for new event notification") | |
| return | |
| } | |
| // Group users by language | |
| languageGroups := make(map[string][]models.NotificationTargetWithLanguage) | |
| for _, user := range users { | |
| if user.NotificationToken != "" { | |
| langCode := user.LanguageCode | |
| if langCode == "" { | |
| langCode = "fr" // Default to French | |
| } | |
| languageGroups[langCode] = append(languageGroups[langCode], user) | |
| } | |
| } | |
| totalSent := 0 | |
| // Send notifications to each language group | |
| for langCode, langUsers := range languageGroups { | |
| localizer := appI18n.GetLocalizer(langCode) | |
| title := localizer.MustLocalize(&i18n.LocalizeConfig{ | |
| MessageID: "event_notification.new_event_title", | |
| DefaultMessage: &i18n.Message{ | |
| ID: "event_notification.new_event_title", | |
| Other: "New event", | |
| }, | |
| }) | |
| message := localizer.MustLocalize(&i18n.LocalizeConfig{ | |
| MessageID: "event_notification.new_event_message", | |
| TemplateData: map[string]interface{}{ | |
| "EventName": req.Name, | |
| "Location": req.Location, | |
| }, | |
| DefaultMessage: &i18n.Message{ | |
| ID: "event_notification.new_event_message", | |
| Other: "{{.EventName}} - {{.Location}}", | |
| }, | |
| }) | |
| var tokens []string | |
| for _, user := range langUsers { | |
| tokens = append(tokens, user.NotificationToken) | |
| } | |
| payload := models.NotificationPayload{ | |
| NotificationTokens: tokens, | |
| Title: title, | |
| Message: message, | |
| Sound: "default", | |
| ChannelID: "default", | |
| Data: map[string]interface{}{ | |
| "screen": "Events", | |
| "eventId": eventID, | |
| }, | |
| } | |
| if err := h.notificationService.SendPushNotification(payload); err != nil { | |
| log.Printf("Failed to send new event notification to %s users: %v", langCode, err) | |
| continue | |
| } | |
| log.Printf("Successfully sent new event notification to %d users in %s", len(tokens), langCode) | |
| totalSent += len(tokens) | |
| } | |
| log.Printf("Successfully sent new event notification to %d users across %d languages", totalSent, len(languageGroups)) | |
| }() |
| // Build query dynamically with correct parameter numbering | ||
| columns := []string{"name", "id_clubs"} | ||
| placeholders := []string{"$1", "$2"} | ||
| args := []interface{}{category.Name, *category.IDClubParent} | ||
| paramNum := 3 | ||
|
|
||
| if category.IDCategoryParent != nil { | ||
| extraCols = ", id_parent_category" | ||
| extraVals = ", $3" | ||
| columns = append(columns, "id_parent_category") | ||
| placeholders = append(placeholders, fmt.Sprintf("$%d", paramNum)) | ||
| args = append(args, *category.IDCategoryParent) | ||
| paramNum++ | ||
| } |
There was a problem hiding this comment.
Potential nil pointer dereference on IDClubParent.
Line 120 dereferences *category.IDClubParent without checking if it's nil. If the caller passes a request without IDClubParent, this will panic.
🐛 Add nil check
func (r *ReservationRepository) CreateCategory(category models.ReservationCreateCategoryRequest) (models.ReservationCategoryComplete, error) {
var res models.ReservationCategoryComplete
if category.Name == "" {
utils.LogMessage(utils.LevelError, "Category name is required")
return res, fmt.Errorf("category name is required")
}
+ if category.IDClubParent == nil {
+ utils.LogMessage(utils.LevelError, "Club parent ID is required")
+ return res, fmt.Errorf("club parent ID is required")
+ }
// Build query dynamically with correct parameter numbering
columns := []string{"name", "id_clubs"}
placeholders := []string{"$1", "$2"}
args := []interface{}{category.Name, *category.IDClubParent}🤖 Prompt for AI Agents
In @handlers/reservation/repository/reservation_repository.go around lines 117 -
128, The code dereferences *category.IDClubParent when building args
(columns/placeholders/args/paramNum) which can panic if IDClubParent is nil;
update the logic in the block that initializes columns/placeholders/args so you
first check if category.IDClubParent != nil before appending and using
*category.IDClubParent, and if that field is required return a validation error
(or if optional, append a nil value or omit the column entirely) so that
functions using columns/placeholders/args (and variables paramNum,
category.Name) never perform a direct dereference of a potentially nil pointer.
| // Build query dynamically with correct parameter numbering | ||
| columns := []string{"name", "slot", "id_clubs"} | ||
| placeholders := []string{"$1", "$2", "$3"} | ||
| args := []interface{}{item.Name, item.Slot, *item.IDClubParent} | ||
| extraCols, extraVals := "", "" | ||
| paramNum := 4 |
There was a problem hiding this comment.
Same nil pointer dereference risk for IDClubParent in CreateItem.
Line 163 dereferences *item.IDClubParent without a nil check.
🐛 Add nil check
func (r *ReservationRepository) CreateItem(item models.ReservationCreateItemRequest) (models.ReservationItemComplete, error) {
var res models.ReservationItemComplete
if item.Name == "" {
utils.LogMessage(utils.LevelError, "Item name is required")
return res, fmt.Errorf("item name is required")
}
+ if item.IDClubParent == nil {
+ utils.LogMessage(utils.LevelError, "Club parent ID is required")
+ return res, fmt.Errorf("club parent ID is required")
+ }
// Build query dynamically with correct parameter numbering
columns := []string{"name", "slot", "id_clubs"}📝 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.
| // Build query dynamically with correct parameter numbering | |
| columns := []string{"name", "slot", "id_clubs"} | |
| placeholders := []string{"$1", "$2", "$3"} | |
| args := []interface{}{item.Name, item.Slot, *item.IDClubParent} | |
| extraCols, extraVals := "", "" | |
| paramNum := 4 | |
| if item.Name == "" { | |
| utils.LogMessage(utils.LevelError, "Item name is required") | |
| return res, fmt.Errorf("item name is required") | |
| } | |
| if item.IDClubParent == nil { | |
| utils.LogMessage(utils.LevelError, "Club parent ID is required") | |
| return res, fmt.Errorf("club parent ID is required") | |
| } | |
| // Build query dynamically with correct parameter numbering | |
| columns := []string{"name", "slot", "id_clubs"} | |
| placeholders := []string{"$1", "$2", "$3"} | |
| args := []interface{}{item.Name, item.Slot, *item.IDClubParent} | |
| paramNum := 4 |
🤖 Prompt for AI Agents
In @handlers/reservation/repository/reservation_repository.go around lines 160 -
164, The code dereferences *item.IDClubParent when building args
(columns/placeholders/args/paramNum) which can panic; in the CreateItem function
add an explicit nil check for item.IDClubParent before using *item.IDClubParent
and handle it (e.g., return a descriptive error from CreateItem such as "missing
IDClubParent" or validate earlier) rather than dereferencing; update any logic
that builds columns/placeholders/args (variables columns, placeholders, args,
paramNum) to only use *item.IDClubParent after the nil check so the function
cannot panic.
| func (es *EventScheduler) checkAndSendEventNotifications() { | ||
| // Get current time + 1 hour | ||
| oneHourFromNow := time.Now().Add(1 * time.Hour) | ||
| // Use a 2-minute window to account for timing variations | ||
| windowStart := oneHourFromNow.Add(-1 * time.Minute) | ||
| windowEnd := oneHourFromNow.Add(1 * time.Minute) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the repository structure
git ls-files | grep -E "(migration|db)" | head -20Repository: PlugImt/transat-backend
Length of output: 923
🏁 Script executed:
# Check for timestamp definitions in migrations
find . -type f -name "*migration*" -o -name "*sql" | head -20Repository: PlugImt/transat-backend
Length of output: 994
🏁 Script executed:
# Search for how start_date is used in database queries
rg -n 'start_date' --type-listRepository: PlugImt/transat-backend
Length of output: 456
🏁 Script executed:
# Check the events migration to see how start_date is defined
cat db/migrations/00014_events.sqlRepository: PlugImt/transat-backend
Length of output: 574
🏁 Script executed:
# Also check the end_date migration
cat db/migrations/00018_non_nullable_event_end_date.sqlRepository: PlugImt/transat-backend
Length of output: 486
🏁 Script executed:
# Search for start_date usage in the codebase
rg -n 'start_date' --type goRepository: PlugImt/transat-backend
Length of output: 4260
🏁 Script executed:
# Look at the full EventScheduler implementation
cat -n scheduler/event_scheduler.goRepository: PlugImt/transat-backend
Length of output: 8595
🏁 Script executed:
# Check the initial schema for the events table definition
cat db/migrations/00001_initial_schema.sql | grep -A 20 "CREATE TABLE events"Repository: PlugImt/transat-backend
Length of output: 1845
🏁 Script executed:
# Search for any other timezone-related configurations or migrations
rg -i 'timezone|time zone' db/migrations/Repository: PlugImt/transat-backend
Length of output: 178
🏁 Script executed:
# Check how event timestamps are inserted into the database to understand timezone handling
rg -n 'INSERT INTO events' -A 5 handlers/Repository: PlugImt/transat-backend
Length of output: 1934
Use UTC consistently for server and database time comparisons.
time.Now() uses the server's local timezone, but start_date is stored as TIMESTAMP (timezone-naive) in the database. If the server timezone differs from the database server's timezone, events will be missed or notified at incorrect times. The query also uses PostgreSQL's NOW(), creating a mismatch between Go's local time values and database comparisons.
Change time.Now() to time.Now().UTC() and ensure the database stores event timestamps in UTC or migrates start_date to TIMESTAMP WITH TIME ZONE for consistency.
Proposed fix
func (es *EventScheduler) checkAndSendEventNotifications() {
// Get current time + 1 hour
- oneHourFromNow := time.Now().Add(1 * time.Hour)
+ oneHourFromNow := time.Now().UTC().Add(1 * time.Hour)
// Use a 2-minute window to account for timing variations
windowStart := oneHourFromNow.Add(-1 * time.Minute)
windowEnd := oneHourFromNow.Add(1 * time.Minute)Additionally, consider migrating the events.start_date column from TIMESTAMP to TIMESTAMP WITH TIME ZONE to eliminate ambiguity.
📝 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.
| func (es *EventScheduler) checkAndSendEventNotifications() { | |
| // Get current time + 1 hour | |
| oneHourFromNow := time.Now().Add(1 * time.Hour) | |
| // Use a 2-minute window to account for timing variations | |
| windowStart := oneHourFromNow.Add(-1 * time.Minute) | |
| windowEnd := oneHourFromNow.Add(1 * time.Minute) | |
| func (es *EventScheduler) checkAndSendEventNotifications() { | |
| // Get current time + 1 hour | |
| oneHourFromNow := time.Now().UTC().Add(1 * time.Hour) | |
| // Use a 2-minute window to account for timing variations | |
| windowStart := oneHourFromNow.Add(-1 * time.Minute) | |
| windowEnd := oneHourFromNow.Add(1 * time.Minute) | |
🤖 Prompt for AI Agents
In @scheduler/event_scheduler.go around lines 58 - 64, In
EventScheduler.checkAndSendEventNotifications replace time.Now() with
time.Now().UTC() when computing oneHourFromNow/windowStart/windowEnd (variables
oneHourFromNow, windowStart, windowEnd) so Go-side times are in UTC, and update
the DB side to use UTC-aware comparisons (either ensure events.start_date is
stored in UTC or migrate start_date to TIMESTAMP WITH TIME ZONE and use
timezone-aware NOW() in queries) to avoid server vs DB timezone mismatches.
| // markNotificationSent marks that a notification has been sent for an event | ||
| func (es *EventScheduler) markNotificationSent(eventID int, notificationType string) { | ||
| // Create table if it doesn't exist (should be done via migration, but this is a safety check) | ||
| createTableQuery := ` | ||
| CREATE TABLE IF NOT EXISTS event_notification_sent ( | ||
| id SERIAL PRIMARY KEY, | ||
| id_events INTEGER NOT NULL, | ||
| notification_type VARCHAR(50) NOT NULL, | ||
| sent_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP, | ||
| UNIQUE(id_events, notification_type), | ||
| FOREIGN KEY (id_events) REFERENCES events(id_events) ON DELETE CASCADE | ||
| ) | ||
| ` | ||
| _, err := es.db.Exec(createTableQuery) | ||
| if err != nil { | ||
| log.Printf("Error creating event_notification_sent table: %v", err) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "*.sql" -o -name "*migration*" -type f | head -20Repository: PlugImt/transat-backend
Length of output: 994
🏁 Script executed:
rg 'event_notification_sent' --type sql --type goRepository: PlugImt/transat-backend
Length of output: 1127
🏁 Script executed:
cat db/migrations/00022_event_notification_sent.sqlRepository: PlugImt/transat-backend
Length of output: 1029
Remove defensive CREATE TABLE IF NOT EXISTS from application code.
Schema changes should only be managed via migrations. The migration 00022_event_notification_sent.sql already handles complete table creation with indexes and proper rollback logic. This defensive check in the application code is redundant and creates several issues:
- Race conditions if multiple instances run simultaneously
- Schema drift if the inline definition diverges from migrations
- Bypasses migration tracking and rollback capabilities
Proposed fix
func (es *EventScheduler) markNotificationSent(eventID int, notificationType string) {
- // Create table if it doesn't exist (should be done via migration, but this is a safety check)
- createTableQuery := `
- CREATE TABLE IF NOT EXISTS event_notification_sent (
- id SERIAL PRIMARY KEY,
- id_events INTEGER NOT NULL,
- notification_type VARCHAR(50) NOT NULL,
- sent_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP,
- UNIQUE(id_events, notification_type),
- FOREIGN KEY (id_events) REFERENCES events(id_events) ON DELETE CASCADE
- )
- `
- _, err := es.db.Exec(createTableQuery)
- if err != nil {
- log.Printf("Error creating event_notification_sent table: %v", err)
- }
-
// Insert or update the notification record
insertQuery := `📝 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.
| // markNotificationSent marks that a notification has been sent for an event | |
| func (es *EventScheduler) markNotificationSent(eventID int, notificationType string) { | |
| // Create table if it doesn't exist (should be done via migration, but this is a safety check) | |
| createTableQuery := ` | |
| CREATE TABLE IF NOT EXISTS event_notification_sent ( | |
| id SERIAL PRIMARY KEY, | |
| id_events INTEGER NOT NULL, | |
| notification_type VARCHAR(50) NOT NULL, | |
| sent_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP, | |
| UNIQUE(id_events, notification_type), | |
| FOREIGN KEY (id_events) REFERENCES events(id_events) ON DELETE CASCADE | |
| ) | |
| ` | |
| _, err := es.db.Exec(createTableQuery) | |
| if err != nil { | |
| log.Printf("Error creating event_notification_sent table: %v", err) | |
| } | |
| // markNotificationSent marks that a notification has been sent for an event | |
| func (es *EventScheduler) markNotificationSent(eventID int, notificationType string) { | |
| // Insert or update the notification record | |
| insertQuery := ` |
🤖 Prompt for AI Agents
In @scheduler/event_scheduler.go around lines 208 - 224, Remove the defensive
CREATE TABLE logic from markNotificationSent on EventScheduler: the inline
createTableQuery and the es.db.Exec call that attempts to create
event_notification_sent should be deleted so schema is managed only via
migrations (migration 00022_event_notification_sent.sql); ensure
markNotificationSent only performs the insert/update logic for recording a sent
notification and does not attempt any DDL, and remove or adjust any related
error logging in that function that referenced the table creation attempt.
* Add allergens support to restaurant menu Introduces database tables for allergens and their association with restaurant articles, including initial allergen data. Adds example HTML pages and updates Go backend models, repositories, and services to support allergen information for menu items. * Update restaurant_scheduler.go
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scheduler/restaurant_scheduler.go (1)
75-76: Remove debug print statements.These
fmt.Printlncalls appear to be debug artifacts and should be removed or converted to proper logging usingutils.LogMessage(utils.LevelDebug, ...)for production code.🧹 Proposed fix
func (s *RestaurantScheduler) isScheduledTimeAllowed(t time.Time) bool { parisTime := utils.ToParisTime(t) weekday := parisTime.Weekday() - fmt.Println("-------> Weekday:", weekday) - fmt.Println("-------> Hour Paris time:", parisTime.Hour()) if weekday == time.Saturday || weekday == time.Sunday { return false }
🤖 Fix all issues with AI agents
In `@handlers/restaurant/repository/menu_repository.go`:
- Around line 145-182: After iterating allergenRows in the function that builds
the allergens slice, add a post-iteration error check by calling
allergenRows.Err() and logging/handling any error returned (similar to existing
logging pattern using utils.LogMessage and fmt.Sprintf). Specifically, after the
for allergenRows.Next() loop for models.Allergen, check if err :=
allergenRows.Err(); err != nil { utils.LogMessage(utils.LevelWarn,
fmt.Sprintf("Failed during allergen rows iteration for dish %d: %v", dishID,
err)) } so any iteration errors on allergenRows are surfaced and handled.
In `@scheduler/restaurant_scheduler.go`:
- Line 82: Log messages still say "9h-14h" while the service window was changed
to 8h-15h; update every hardcoded occurrence of the string "9h-14h" in
restaurant_scheduler.go to "8h-15h" (the two occurrences near the scheduler
start/stop logs) so logs match the actual logic that returns hour >= 8 && hour <
15; ensure any surrounding text remains grammatically correct and update any
related test or comment strings if present.
🧹 Nitpick comments (2)
models/restaurant.go (1)
52-61: Consider using a slice for allergen fields if the API structure permits.The 10 individual
Allergene1throughAllergene10fields work but require repetitive handling. If the external API cannot be changed, this is acceptable; otherwise, an array would simplify processing.handlers/restaurant/service/menu_service.go (1)
136-143: Consider extractingcategoryToIDmap to avoid duplication.This map is defined in three places:
BuildFetchedItems(lines 136-143)ConvertMenuDataToFetchedItems(lines 226-233)- Implicitly in
processRawMenuItemsswitch statementExtracting it to a package-level constant would reduce maintenance burden.
Example extraction
// At package level var categoryToID = map[string]int{ "grilladesMidi": 1, "migrateurs": 2, "cibo": 3, "accompMidi": 4, "grilladesSoir": 5, "accompSoir": 6, }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
db/migrations/00023_allergens.sqlexemple_restaurant_page.htmlexemple_restaurant_page_wrapped.htmlhandlers/restaurant/repository/menu_repository.gohandlers/restaurant/scheduler.gohandlers/restaurant/service/menu_service.gomodels/restaurant.goscheduler/restaurant_scheduler.go
🧰 Additional context used
🧬 Code graph analysis (3)
handlers/restaurant/repository/menu_repository.go (2)
utils/logManager.go (2)
LogMessage(62-64)LevelWarn(17-17)models/restaurant.go (3)
Allergen(35-41)DishDetailsResponse(146-153)RestaurantArticle(12-17)
handlers/restaurant/scheduler.go (2)
handlers/restaurant/service/menu_service.go (1)
MenuService(15-18)utils/logManager.go (3)
LogMessage(62-64)LevelError(16-16)LogFooter(70-72)
handlers/restaurant/service/menu_service.go (1)
models/restaurant.go (3)
MenuItemAPI(45-62)MenuData(73-80)FetchedItem(65-70)
🔇 Additional comments (11)
handlers/restaurant/scheduler.go (1)
29-57: LGTM! Clean refactor to support allergen data flow.The updated
CheckAndUpdateMenuCroncorrectly chains:
FetchRawMenuItems()to get raw API data with allergen fieldsBuildFetchedItems()to transform and preserve allergen codesSyncTodaysMenu()for database synchronizationError handling and logging remain consistent with the previous implementation.
db/migrations/00023_allergens.sql (3)
5-12: Schema design looks solid.The
allergenstable structure withnameas UNIQUE constraint enables safe ON CONFLICT handling during seeding and runtime lookups by code.
14-28: FK cascade strategy is appropriate.
ON DELETE CASCADEonid_restaurant_articles: removes allergen links when an article is deletedON DELETE RESTRICTonid_allergen: prevents accidental deletion of allergens that are still referencedThis protects referential integrity while allowing article cleanup.
39-102: Clarify where pictosAllergenes assets are located and served.The
picture_urlvalues store relative paths likeviews/assets/img/pictosAllergenes/Arachide.svg, but these assets don't exist in this backend repository. These are metadata references that the backend returns via the API for clients to consume. Ensure:
- Frontend/client applications know where these assets are actually hosted (separate frontend repo, CDN, etc.)
- The path format aligns with how the frontend or deployment infrastructure serves static assets
- Asset availability is verified in the frontend deployment, not the backend
models/restaurant.go (2)
34-41: LGTM! Allergen model correctly maps to DB schema.The struct fields and tags align with the migration schema (
id_allergen,name,description,description_en,picture_url).
144-153: LGTM! Well-structured response type.
DishDetailsResponsecleanly embedsRestaurantArticleand extends it with computed fields (AverageRating,TotalRatings,TimesServed) and related data (Allergens,Reviews). The JSON field namerecent_reviewsforReviewsappropriately describes the content.handlers/restaurant/repository/menu_repository.go (2)
501-540: Allergen sync logic handles edge cases well.The implementation:
- Creates missing allergens on-the-fly (with minimal data)
- Uses
ON CONFLICT DO NOTHINGfor idempotent article-allergen linking- Logs warnings without blocking the sync process
One consideration: dynamically inserted allergens (line 515-523) will lack
description,description_en, andpicture_url. This is acceptable if all expected allergens are pre-seeded, but could result in incomplete data for unknown codes.
196-209: Good graceful degradation on review fetch failure.Returning partial data with allergens but nil reviews when the reviews query fails is a reasonable approach that maintains API availability.
handlers/restaurant/service/menu_service.go (3)
31-71: LGTM! Clean extraction of raw API fetching.
FetchRawMenuItemsproperly separates the HTTP fetch and JSON parsing from the data transformation, enabling the new allergen-preserving flow while maintaining backward compatibility viaFetchMenuFromAPI.
162-182: Allergen code collection is well-implemented.The logic correctly:
- Trims whitespace from each code
- Filters out empty strings and placeholder values (
vide,undefined)- Deduplicates codes using a seen map
- Preserves original casing for DB lookup
131-193: Note:BuildFetchedItemsallows duplicate item names unlikeprocessRawMenuItems.
processRawMenuItemsuses anitemMapto deduplicate items by category+name, butBuildFetchedItemsdoes not. This is acceptable sinceSyncTodaysMenuusesON CONFLICTat the database level, but be aware that duplicate API entries will result in redundant processing and allergen sync operations.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // Fetch allergens linked to this article | ||
| allergensQuery := ` | ||
| SELECT a.id_allergen, | ||
| a.name, | ||
| COALESCE(a.description, ''), | ||
| COALESCE(a.description_en, ''), | ||
| COALESCE(a.picture_url, '') | ||
| FROM restaurant_article_allergens raa | ||
| JOIN allergens a ON raa.id_allergen = a.id_allergen | ||
| WHERE raa.id_restaurant_articles = $1 | ||
| ORDER BY a.name; | ||
| ` | ||
|
|
||
| allergenRows, err := r.DB.Query(allergensQuery, dishID) | ||
| if err != nil { | ||
| utils.LogMessage(utils.LevelWarn, fmt.Sprintf("Failed to fetch allergens for dish %d: %v", dishID, err)) | ||
| } | ||
|
|
||
| var allergens []models.Allergen | ||
| if allergenRows != nil { | ||
| defer func(rows *sql.Rows) { | ||
| if rows == nil { | ||
| return | ||
| } | ||
| if err := rows.Close(); err != nil { | ||
| utils.LogMessage(utils.LevelWarn, fmt.Sprintf("Failed to close allergen rows: %v", err)) | ||
| } | ||
| }(allergenRows) | ||
|
|
||
| for allergenRows.Next() { | ||
| var a models.Allergen | ||
| if err := allergenRows.Scan(&a.ID, &a.Name, &a.Description, &a.DescriptionEn, &a.PictureURL); err != nil { | ||
| utils.LogMessage(utils.LevelWarn, fmt.Sprintf("Failed to scan allergen: %v", err)) | ||
| continue | ||
| } | ||
| allergens = append(allergens, a) | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing allergenRows.Err() check after iteration.
After the for allergenRows.Next() loop, you should check allergenRows.Err() to catch any errors that occurred during iteration. This is a common Go database/sql best practice.
Proposed fix
for allergenRows.Next() {
var a models.Allergen
if err := allergenRows.Scan(&a.ID, &a.Name, &a.Description, &a.DescriptionEn, &a.PictureURL); err != nil {
utils.LogMessage(utils.LevelWarn, fmt.Sprintf("Failed to scan allergen: %v", err))
continue
}
allergens = append(allergens, a)
}
+ if err := allergenRows.Err(); err != nil {
+ utils.LogMessage(utils.LevelWarn, fmt.Sprintf("Error iterating allergen rows: %v", err))
+ }
}📝 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.
| // Fetch allergens linked to this article | |
| allergensQuery := ` | |
| SELECT a.id_allergen, | |
| a.name, | |
| COALESCE(a.description, ''), | |
| COALESCE(a.description_en, ''), | |
| COALESCE(a.picture_url, '') | |
| FROM restaurant_article_allergens raa | |
| JOIN allergens a ON raa.id_allergen = a.id_allergen | |
| WHERE raa.id_restaurant_articles = $1 | |
| ORDER BY a.name; | |
| ` | |
| allergenRows, err := r.DB.Query(allergensQuery, dishID) | |
| if err != nil { | |
| utils.LogMessage(utils.LevelWarn, fmt.Sprintf("Failed to fetch allergens for dish %d: %v", dishID, err)) | |
| } | |
| var allergens []models.Allergen | |
| if allergenRows != nil { | |
| defer func(rows *sql.Rows) { | |
| if rows == nil { | |
| return | |
| } | |
| if err := rows.Close(); err != nil { | |
| utils.LogMessage(utils.LevelWarn, fmt.Sprintf("Failed to close allergen rows: %v", err)) | |
| } | |
| }(allergenRows) | |
| for allergenRows.Next() { | |
| var a models.Allergen | |
| if err := allergenRows.Scan(&a.ID, &a.Name, &a.Description, &a.DescriptionEn, &a.PictureURL); err != nil { | |
| utils.LogMessage(utils.LevelWarn, fmt.Sprintf("Failed to scan allergen: %v", err)) | |
| continue | |
| } | |
| allergens = append(allergens, a) | |
| } | |
| } | |
| // Fetch allergens linked to this article | |
| allergensQuery := ` | |
| SELECT a.id_allergen, | |
| a.name, | |
| COALESCE(a.description, ''), | |
| COALESCE(a.description_en, ''), | |
| COALESCE(a.picture_url, '') | |
| FROM restaurant_article_allergens raa | |
| JOIN allergens a ON raa.id_allergen = a.id_allergen | |
| WHERE raa.id_restaurant_articles = $1 | |
| ORDER BY a.name; | |
| ` | |
| allergenRows, err := r.DB.Query(allergensQuery, dishID) | |
| if err != nil { | |
| utils.LogMessage(utils.LevelWarn, fmt.Sprintf("Failed to fetch allergens for dish %d: %v", dishID, err)) | |
| } | |
| var allergens []models.Allergen | |
| if allergenRows != nil { | |
| defer func(rows *sql.Rows) { | |
| if rows == nil { | |
| return | |
| } | |
| if err := rows.Close(); err != nil { | |
| utils.LogMessage(utils.LevelWarn, fmt.Sprintf("Failed to close allergen rows: %v", err)) | |
| } | |
| }(allergenRows) | |
| for allergenRows.Next() { | |
| var a models.Allergen | |
| if err := allergenRows.Scan(&a.ID, &a.Name, &a.Description, &a.DescriptionEn, &a.PictureURL); err != nil { | |
| utils.LogMessage(utils.LevelWarn, fmt.Sprintf("Failed to scan allergen: %v", err)) | |
| continue | |
| } | |
| allergens = append(allergens, a) | |
| } | |
| if err := allergenRows.Err(); err != nil { | |
| utils.LogMessage(utils.LevelWarn, fmt.Sprintf("Error iterating allergen rows: %v", err)) | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@handlers/restaurant/repository/menu_repository.go` around lines 145 - 182,
After iterating allergenRows in the function that builds the allergens slice,
add a post-iteration error check by calling allergenRows.Err() and
logging/handling any error returned (similar to existing logging pattern using
utils.LogMessage and fmt.Sprintf). Specifically, after the for
allergenRows.Next() loop for models.Allergen, check if err :=
allergenRows.Err(); err != nil { utils.LogMessage(utils.LevelWarn,
fmt.Sprintf("Failed during allergen rows iteration for dish %d: %v", dishID,
err)) } so any iteration errors on allergenRows are surfaced and handled.
scheduler/restaurant_scheduler.go
Outdated
|
|
||
| hour := parisTime.Hour() | ||
| return hour >= 8 && hour < 14 | ||
| return hour >= 8 && hour < 15 |
There was a problem hiding this comment.
Time window extended; update log messages for consistency.
The time window is now 8h-15h (Paris time), but log messages on lines 50 and 89 still reference "9h-14h". Update them to reflect the actual window.
🔧 Proposed fix for log messages
@@ -47,7 +47,7 @@
case <-ticker.C:
if s.isScheduledTimeAllowed(time.Now()) {
go s.checkAndUpdateMenu()
} else {
- utils.LogMessage(utils.LevelDebug, "Scheduler: Outside allowed time window (weekdays 9h-14h), skipping check")
+ utils.LogMessage(utils.LevelDebug, "Scheduler: Outside allowed time window (weekdays 8h-15h), skipping check")
}@@ -86,7 +86,7 @@
func (s *RestaurantScheduler) checkAndUpdateMenu() {
now := time.Now()
if !s.isScheduledTimeAllowed(now) {
- utils.LogMessage(utils.LevelInfo, "Scheduler: Outside allowed time window (weekdays 9h-14h), skipping menu check")
+ utils.LogMessage(utils.LevelInfo, "Scheduler: Outside allowed time window (weekdays 8h-15h), skipping menu check")
return
}📝 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.
| return hour >= 8 && hour < 15 | |
| } else { | |
| utils.LogMessage(utils.LevelDebug, "Scheduler: Outside allowed time window (weekdays 8h-15h), skipping check") | |
| } |
| return hour >= 8 && hour < 15 | |
| if !s.isScheduledTimeAllowed(now) { | |
| utils.LogMessage(utils.LevelInfo, "Scheduler: Outside allowed time window (weekdays 8h-15h), skipping menu check") | |
| return | |
| } |
🤖 Prompt for AI Agents
In `@scheduler/restaurant_scheduler.go` at line 82, Log messages still say
"9h-14h" while the service window was changed to 8h-15h; update every hardcoded
occurrence of the string "9h-14h" in restaurant_scheduler.go to "8h-15h" (the
two occurrences near the scheduler start/stop logs) so logs match the actual
logic that returns hour >= 8 && hour < 15; ensure any surrounding text remains
grammatically correct and update any related test or comment strings if present.
Updated the isScheduledTimeAllowed function to allow scheduling until 16:00 instead of 15:00. Also made minor formatting adjustments in models/restaurant.go for consistency.
Introduces an is_marker boolean column to the allergens table to distinguish between allergens and label/marker entries. Updates the Allergen model to include the is_marker field and migrates marker pictograms into the allergens table with full absolute URLs for picture_url.
Introduces handling of marker fields (e.g., bio, local, vegetarien) from the API in the menu sync process. Updates models to include marker fields, extracts marker codes in the service layer, and syncs marker associations in the repository. This allows markers to be stored and linked similarly to allergens.
cf pr front
ajout de notifs pour les events
Summary by CodeRabbit
New Features
Configuration
Database
✏️ Tip: You can customize this high-level summary in your review settings.