Verse of day post cms#230
Conversation
Confidence Score: 3/5Two confirmed defects should be resolved before merging: image removal is broken in edit mode, and expanding the shared language constant breaks Group and Series forms in a non-obvious way. The edit-mode image removal bug means the server retains images the user intended to delete. The PLAN_LANGUAGE expansion is a cross-cutting change — Group and Series forms will now surface HI/NE/MN language tabs, but their Zod schemas only accept EN/BO/ZH, so user-entered content in those new tabs is silently discarded on submit. src/lib/constant.ts (shared language constant affects multiple schemas) and src/components/routes/verse-of-day/VerseOfDayForm.tsx (image removal logic in the update path) Reviews (2): Last reviewed commit: "language_support_expansion" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /> | ||
| </div> | ||
| <Button | ||
| variant="outline" | ||
| className="bg-gray-100 hover:bg-gray-200" | ||
| onClick={handleOpenCreate} | ||
| > |
There was a problem hiding this comment.
debouncedSearch is placed in the query key (line 42) and that changes which cache entry the parent reads, but two problems follow: (1) fetchVerseOfDayList ignores all arguments and always returns the full data set, so the extra cache key segment never produces filtered data; (2) VerseOfDayList makes its own independent query with key ["verse-of-day-list"] (no search param) and renders from that result — entirely bypassing the parent's filtered state. A user typing in this search box will see no change in the table.
| <div className="border w-fit px-2 bg-white dark:bg-input/30 rounded-md border-gray-200 dark:border-[#313132] flex items-center"> | ||
| <IoMdSearch className="w-4 h-4" /> | ||
| <Pecha.Input | ||
| placeholder="Search verses..." | ||
| className="rounded-md border-none dark:bg-transparent px-4 shadow-none py-2" | ||
| value={search} | ||
| onChange={(e) => { | ||
| setSearch(e.target.value); | ||
| if (currentPage !== 1) setCurrentPage(1); | ||
| }} | ||
| /> | ||
| </div> | ||
| <Button | ||
| variant="outline" | ||
| className="bg-gray-100 hover:bg-gray-200" | ||
| onClick={handleOpenCreate} | ||
| > | ||
| <IoMdAdd /> Add Verse | ||
| </Button> | ||
| </div> | ||
| <AuthButton /> | ||
| </div> | ||
|
|
||
| <div className="flex-1 overflow-hidden px-6 py-4"> | ||
| {error ? ( | ||
| <p className="text-sm text-red-500 py-8"> | ||
| Failed to load verses. {getApiErrorMessage(error)} | ||
| </p> | ||
| ) : sortedVerses.length === 0 && !isLoading ? ( | ||
| <div className="flex flex-col h-full items-center justify-center"> | ||
| <p className="text-base text-muted-foreground">No verses found</p> | ||
| <Button | ||
| variant="outline" | ||
| className="mt-2" | ||
| onClick={handleOpenCreate} | ||
| > | ||
| <IoMdAdd /> Add Verse | ||
| </Button> | ||
| </div> | ||
| ) : ( | ||
| <div className="h-full overflow-auto"> | ||
| <VerseOfDayList | ||
| onEdit={handleOpenEdit} | ||
| onDelete={setDeleteTarget} | ||
| /> | ||
| </div> | ||
| )} | ||
| </div> | ||
|
|
||
| {sortedVerses.length > 0 && ( | ||
| <div className="border-t border-gray-200 dark:border-[#313132] px-6 py-4 bg-white dark:bg-[#1E1E1E]"> | ||
| <Pagination | ||
| currentPage={currentPage} | ||
| totalPages={totalPages} | ||
| onPageChange={setCurrentPage} | ||
| /> | ||
| </div> | ||
| )} | ||
|
|
||
| <VerseOfDayFormDialog | ||
| open={formOpen} | ||
| onOpenChange={(open: boolean) => { | ||
| setFormOpen(open); |
There was a problem hiding this comment.
totalPages and currentPage are computed/stored in the parent, but VerseOfDayList renders its own independent query (key ["verse-of-day-list"]) and always displays the full set of verses. The Pagination component only changes the parent's currentPage state and query-key, which never influences what the list actually shows. The PAGE_SIZE constant is effectively dead code in the current design.
| fetchVerseOfDayList, | ||
| type VerseOfDayItem, | ||
| } from "./api/verseOfDayApi"; | ||
| import { format } from "date-fns"; |
There was a problem hiding this comment.
new Date("2024-01-15") parses an ISO date-only string as UTC midnight. When date-fns format renders it in a timezone behind UTC (e.g., US Eastern at UTC−5), midnight UTC on Jan 15 is 7 PM local time on Jan 14, so the cell will display "Jan 14" instead of "Jan 15". Using date-fns parse with the same format string creates a local-midnight Date and avoids the shift.
| import { format } from "date-fns"; | |
| import { format, parse } from "date-fns"; |
| <div className={`h-12 w-12 rounded bg-muted ${verse.image_url ? 'hidden' : ''}`} /> | ||
| </Pecha.TableCell> | ||
| <Pecha.TableCell className="whitespace-nowrap"> | ||
| {format(new Date(verse.date), "MMM dd, yyyy")} |
There was a problem hiding this comment.
new Date(verse.date) with parse(verse.date, "yyyy-MM-dd", new Date()) to produce a local-midnight Date and avoid the UTC off-by-one day issue described above.
| {format(new Date(verse.date), "MMM dd, yyyy")} | |
| {format(parse(verse.date, "yyyy-MM-dd", new Date()), "MMM dd, yyyy")} |
| const getAuthHeaders = () => ({ | ||
| Authorization: `Bearer ${sessionStorage.getItem("accessToken")}`, | ||
| }); | ||
|
|
||
| export const fetchVerseOfDayList = async (): Promise<VerseOfDayListResponse> => { | ||
| const { data } = await axiosInstance.get<VerseOfDayListResponse>( | ||
| `/api/v1/cms/verse-of-day`, | ||
| { | ||
| headers: getAuthHeaders(), | ||
| }, | ||
| ); | ||
| return data; |
There was a problem hiding this comment.
axiosInstance already has a request interceptor (in axios-config.ts) that reads sessionStorage.getItem("accessToken") and sets Authorization: Bearer <token> on every request. The getAuthHeaders() helper duplicates this for every call in this file. It is harmless but inconsistent with all other API files in the codebase (e.g., tagsApi.ts, groupsApi.ts), which rely solely on the interceptor. The helper and its usages can be removed.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| {pickGroupTitle(group.metadata)} | ||
| </Pecha.CommandItem> | ||
| ))} | ||
| </Pecha.CommandGroup> | ||
| </Pecha.CommandList> | ||
| </Pecha.Command> | ||
| </Pecha.PopoverContent> | ||
| </Pecha.Popover> |
There was a problem hiding this comment.
When a user clicks "remove image" in edit mode, handleRemoveImage sets both imageKey and imagePreview to null. The update payload block only adds image_urls when imageKey is truthy — so removing an existing image never sends any value to the API. The server keeps the old image_url, but the UI shows it as removed. To signal deletion the update payload should explicitly send image_urls: [] when the image was removed (i.e. imagePreview is null but initialData.image_url was set).
2 participants
No description provided.