-
Notifications
You must be signed in to change notification settings - Fork 0
feat(ui): refactor location form component #71
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
Conversation
- Introduce a new LocationForm component for handling location input and editing - Add support for location submission, validation, and error handling - Implement coordinate selection via map interaction and search - Add loading states and form validation - Include success/error feedback and navigation handling refactor: update location handling and related components - Replace old location form implementation with new LocationForm component - Improve location page routing and navigation - Add support for location editing and updates - Enhance error handling and form validation - Restructure location-related code and state management feat(api): add location update endpoint - Add new PUT endpoint for updating existing locations - Implement slug-based updates with validation - Add conflict checking for duplicate location names - Support authenticated updates via CSRF protection style: clean up old form implementation - Remove obsolete code from dashboard/add.vue - Replace with LocationForm component - Improve code organization and readability build: update dependencies and imports - Add new imports and dependency management - Update schema references and type definitions - Ensure proper TypeScript support across components
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughIntroduces a reusable LocationForm component with map/validation, adds update API and DB query, refactors dashboard/layout logic using centralized constants, rewires add/edit pages to use LocationForm, and updates slug page rendering and store to use route-name sets and preloading. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant EditPage as Edit Page
participant LocationForm
participant API as PUT /api/locations/:slug
participant DB as updateLocationBySlug
User->>EditPage: Open Edit
EditPage->>LocationForm: Render with initialValues
User->>LocationForm: Edit fields, move map pin
User->>LocationForm: Click Save
LocationForm->>API: Submit validated payload
API->>DB: updateLocationBySlug(values, slug, userId)
DB-->>API: Updated location
API-->>LocationForm: 200 OK
LocationForm->>EditPage: onSubmitComplete()
EditPage->>User: Navigate to location dashboard
sequenceDiagram
actor User
participant AddPage as Add Page
participant LocationForm
participant API as POST /api/location
User->>AddPage: Open Add
AddPage->>LocationForm: Render
User->>LocationForm: Search place / set pin / fill fields
User->>LocationForm: Click Create
LocationForm->>API: Submit validated payload
API-->>LocationForm: Response
LocationForm->>AddPage: onSubmitComplete()
AddPage->>User: Navigate to /dashboard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 9
🔭 Outside diff range comments (1)
stores/locations.ts (1)
46-46
: Remove duplicate push operation.Line 46 duplicates the
mapPoints.push(mapPoint)
operation that already exists on line 38, causing each map point to be added twice to the array.- mapPoints.push(mapPoint);
🧹 Nitpick comments (9)
lib/constant.ts (1)
5-18
: Consider using an enum or object for better type safety and maintainability.While Sets provide good performance for membership checks, consider using an enum or const object with as const assertion for better type safety and IDE support. This would prevent typos and provide better autocompletion.
-export const LOCATION_PAGES = new Set([ - "dashboard", - "dashboard-add", -]); -export const CURRENT_LOCATION_PAGES = new Set([ - "dashboard-location-slug", - "dashboard-location-slug-add", - "dashboard-location-slug-edit", -]); -export const EDIT_PAGES = new Set([ - "dashboard-add", - "dashboard-location-slug-add", - "dashboard-location-slug-edit", -]); +export const PAGE_GROUPS = { + LOCATION: ["dashboard", "dashboard-add"] as const, + CURRENT_LOCATION: [ + "dashboard-location-slug", + "dashboard-location-slug-add", + "dashboard-location-slug-edit" + ] as const, + EDIT: [ + "dashboard-add", + "dashboard-location-slug-add", + "dashboard-location-slug-edit" + ] as const +} as const; + +export const LOCATION_PAGES = new Set(PAGE_GROUPS.LOCATION); +export const CURRENT_LOCATION_PAGES = new Set(PAGE_GROUPS.CURRENT_LOCATION); +export const EDIT_PAGES = new Set(PAGE_GROUPS.EDIT); + +export type PageGroup = typeof PAGE_GROUPS; +export type LocationPage = PageGroup['LOCATION'][number]; +export type CurrentLocationPage = PageGroup['CURRENT_LOCATION'][number]; +export type EditPage = PageGroup['EDIT'][number];stores/locations.ts (1)
33-33
: Add null check for route.name to prevent potential runtime errors.The
route.name
could benull
in certain scenarios. WhiletoString()
onnull
returns"null"
which won't match any page names, it's better to handle this explicitly for clarity.- if (locations.value && LOCATION_PAGES.has(route.name?.toString() || "")) { + if (locations.value && route.name && LOCATION_PAGES.has(route.name.toString())) {- else if (currentLocation.value && CURRENT_LOCATION_PAGES.has(route.name?.toString() || "")) { + else if (currentLocation.value && route.name && CURRENT_LOCATION_PAGES.has(route.name.toString())) {Also applies to: 51-51
pages/dashboard/location/[slug].vue (2)
5-7
: Remove async conversion as it's not needed.The
onMounted
hook doesn't need to be async sincerefreshCurrentLocation()
doesn't return a value that needs to be awaited here.
20-24
: Consider improving error display with more context.The error message display could be enhanced to provide more actionable information to users, such as retry options or navigation suggestions.
- <div v-if="error && status !== 'pending'" class="alert alert-error w-64"> - <h2 class="text-lg"> - {{ error.statusMessage }} - </h2> - </div> + <div v-if="error && status !== 'pending'" class="alert alert-error"> + <Icon name="tabler:alert-circle" size="24" /> + <div> + <h3 class="font-bold">Unable to load location</h3> + <div class="text-sm">{{ error.statusMessage || 'An unexpected error occurred' }}</div> + </div> + <button class="btn btn-sm" @click="locationStore.refreshCurrentLocation()"> + Retry + </button> + </div>pages/dashboard.vue (1)
112-114
: Consider using reactive computed property for loading state.The loading indicator condition could be simplified using a computed property for better reactivity and readability.
Add this to the script section:
const isLoadingCurrentLocation = computed(() => route.path.startsWith('/dashboard/location') && currentLocationStatus.value === 'pending' );Then update the template:
- <div v-if="route.path.startsWith('/dashboard/location') && currentLocationStatus === 'pending'" class="flex items-center justify-center"> + <div v-if="isLoadingCurrentLocation" class="flex items-center justify-center">pages/dashboard/location/[slug]/edit.vue (1)
7-12
: Consider returning the updated entity from onSubmit (optional)LocationForm doesn’t use the return value today, but returning the updated location (from the PUT) can be handy for optimistic UI or store updates if needed later.
components/location-form.vue (3)
60-68
: Initialize map point correctly; consider using a sentinel idUsing a hard-coded
id: 1
could collide if the map store assumes uniqueness. IfaddedPoint
is a single slot, it’s okay; otherwise consider a unique sentinel (e.g., negative orDate.now()
).Optional improvement example:
- id: 1, + id: -1,
69-80
: Clear map state on component unmount tooYou clear
addedPoint
on route leave, but if the component unmounts without a route change (e.g., parent hides it), the state persists. Also clear on unmount.Add outside this hunk:
onUnmounted(() => { mapStore.addedPoint = null; });
146-154
: Nit: punctuation typo in helper text“marker on the map..” has a double period.
Apply this diff:
- /> marker on the map.. + /> marker on the map.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
components/location-form.vue
(1 hunks)lib/constant.ts
(1 hunks)lib/db/queries/location.ts
(1 hunks)pages/dashboard.vue
(4 hunks)pages/dashboard/add.vue
(2 hunks)pages/dashboard/location/[slug].vue
(2 hunks)pages/dashboard/location/[slug]/edit.vue
(1 hunks)server/api/locations/[slug].put.ts
(1 hunks)stores/locations.ts
(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
lib/db/queries/location.ts (1)
lib/db/schema/location.ts (3)
InsertLocation
(28-39)InsertLocation
(41-41)location
(12-22)
stores/locations.ts (1)
lib/constant.ts (2)
LOCATION_PAGES
(5-8)CURRENT_LOCATION_PAGES
(9-13)
server/api/locations/[slug].put.ts (3)
utils/define-authenticated-event-handler.ts (1)
defineAuthenticatedEventHandler
(11-23)utils/send-zod-error.ts (1)
SendZodError
(4-21)lib/db/queries/location.ts (2)
findLocationByName
(28-35)updateLocationBySlug
(79-85)
🔇 Additional comments (4)
pages/dashboard/location/[slug].vue (1)
8-12
: Good implementation of route update handling.The
onBeforeRouteUpdate
hook properly refreshes location data when navigating to the same named route, ensuring data stays current during navigation.lib/db/queries/location.ts (1)
79-85
: No manualupdated_at
handling neededThe
updated_at
column inlib/db/schema/location.ts
(and related tables) is defined with$default(() => Date.now()).$onUpdate(() => Date.now())
. Drizzle ORM will automatically apply the timestamp update on each row update, so no explicitupdated_at
assignment is required inupdateLocationBySlug
.server/api/locations/[slug].put.ts (1)
14-21
: Name conflict check looks correctThe uniqueness check for name conflicts (excluding the current slug) aligns with your DB uniqueness constraint (name, userId). Good guard against duplicates.
components/location-form.vue (1)
82-101
: Good error handling and UX
- Applies server-side validation errors to fields.
- Shows a concise error alert when non-validation errors occur.
- Preserves form state and uses a clear loading flag.
This aligns well with the acceptance criteria.
function formatNumber(value?: number) { | ||
if (!value) | ||
return 0; | ||
return value.toFixed(5); | ||
} |
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.
🛠️ Refactor suggestion
Fix number formatting for 0 and ensure consistent string return
if (!value)
treats 0 as falsy and returns 0 instead of “0.00000”. Also returns mixed types. Always return a string and only treat null/undefined as missing.
Apply this diff:
-function formatNumber(value?: number) {
- if (!value)
- return 0;
- return value.toFixed(5);
-}
+function formatNumber(value?: number) {
+ if (value == null)
+ return "0.00000";
+ return value.toFixed(5);
+}
📝 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.
function formatNumber(value?: number) { | |
if (!value) | |
return 0; | |
return value.toFixed(5); | |
} | |
function formatNumber(value?: number) { | |
if (value == null) | |
return "0.00000"; | |
return value.toFixed(5); | |
} |
🤖 Prompt for AI Agents
In components/location-form.vue around lines 24 to 28, the function treats 0 as
falsy and returns a number instead of a formatted string; change the check to
only treat null/undefined (use value == null), ensure the function returns a
string type, and return value.toFixed(5) for numeric values (return an empty
string or other agreed "missing" string when value is null/undefined) so 0
becomes "0.00000" and all returns are strings.
effect(() => { | ||
if (mapStore.addedPoint) { | ||
setFieldValue("long", mapStore.addedPoint.long); | ||
setFieldValue("lat", mapStore.addedPoint.lat); | ||
} | ||
}); |
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.
Replace non-existent effect
with watchEffect
effect
is not a public Vue composition API in Nuxt/Vue apps; use watchEffect
(auto-imported in Nuxt) to react to mapStore.addedPoint
changes.
Apply this diff:
-effect(() => {
+watchEffect(() => {
if (mapStore.addedPoint) {
setFieldValue("long", mapStore.addedPoint.long);
setFieldValue("lat", mapStore.addedPoint.lat);
}
});
📝 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.
effect(() => { | |
if (mapStore.addedPoint) { | |
setFieldValue("long", mapStore.addedPoint.long); | |
setFieldValue("lat", mapStore.addedPoint.lat); | |
} | |
}); | |
watchEffect(() => { | |
if (mapStore.addedPoint) { | |
setFieldValue("long", mapStore.addedPoint.long); | |
setFieldValue("lat", mapStore.addedPoint.lat); | |
} | |
}); |
🤖 Prompt for AI Agents
In components/location-form.vue around lines 53 to 58, the code uses a
non-existent Vue API named effect; replace it with watchEffect so the
composition API reacts to mapStore.addedPoint changes—i.e. change effect(() => {
... }) to watchEffect(() => { ... }) (Nuxt auto-imports watchEffect), keeping
the same body that calls setFieldValue for long and lat.
export async function updateLocationBySlug(updates: InsertLocation, slug: string, userId: number) { | ||
const [updated] = await db.update(location).set(updates).where(and( | ||
eq(location.slug, slug), | ||
eq(location.userId, userId), | ||
)).returning(); | ||
return updated; | ||
} |
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.
Add validation to ensure update operation succeeded.
The function assumes the update will always return a result, but it might return an empty array if no matching record is found. This should be handled explicitly.
export async function updateLocationBySlug(updates: InsertLocation, slug: string, userId: number) {
- const [updated] = await db.update(location).set(updates).where(and(
+ const result = await db.update(location).set(updates).where(and(
eq(location.slug, slug),
eq(location.userId, userId),
)).returning();
- return updated;
+
+ if (result.length === 0) {
+ return null;
+ }
+
+ return result[0];
}
Then update the calling code to handle the null case appropriately.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export async function updateLocationBySlug(updates: InsertLocation, slug: string, userId: number) { | |
const [updated] = await db.update(location).set(updates).where(and( | |
eq(location.slug, slug), | |
eq(location.userId, userId), | |
)).returning(); | |
return updated; | |
} | |
export async function updateLocationBySlug( | |
updates: InsertLocation, | |
slug: string, | |
userId: number, | |
) { | |
const result = await db | |
.update(location) | |
.set(updates) | |
.where( | |
and( | |
eq(location.slug, slug), | |
eq(location.userId, userId), | |
), | |
) | |
.returning(); | |
if (result.length === 0) { | |
return null; | |
} | |
return result[0]; | |
} |
🤖 Prompt for AI Agents
In lib/db/queries/location.ts around lines 79 to 85, the updateLocationBySlug
function assumes db.update(...).returning() always yields a row; if no record
matches the slug/userId it returns an empty array and the function returns
undefined. Change the function to check the returned array length and explicitly
return null (or throw a descriptive error) when no row was updated, and update
all callers to handle the null case (either propagate the error, return 404 to
clients, or take appropriate fallback action).
if (LOCATION_PAGES.has(route.name?.toString() || "")) { | ||
await locationStore.refreshLocations(); | ||
} | ||
|
||
if (CURRENT_LOCATION_PAGES.has(route.name?.toString() || "")) { | ||
await locationStore.refreshCurrentLocation(); | ||
} |
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.
🛠️ Refactor suggestion
Consider error handling for data refresh operations.
The top-level await calls to refresh data don't have error handling. If these fail, the page might not render properly.
-if (LOCATION_PAGES.has(route.name?.toString() || "")) {
- await locationStore.refreshLocations();
-}
-
-if (CURRENT_LOCATION_PAGES.has(route.name?.toString() || "")) {
- await locationStore.refreshCurrentLocation();
-}
+try {
+ if (LOCATION_PAGES.has(route.name?.toString() || "")) {
+ await locationStore.refreshLocations();
+ }
+
+ if (CURRENT_LOCATION_PAGES.has(route.name?.toString() || "")) {
+ await locationStore.refreshCurrentLocation();
+ }
+} catch (error) {
+ console.error('Failed to refresh location data:', error);
+ // Let the page render anyway - error states will be handled by components
+}
📝 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.
if (LOCATION_PAGES.has(route.name?.toString() || "")) { | |
await locationStore.refreshLocations(); | |
} | |
if (CURRENT_LOCATION_PAGES.has(route.name?.toString() || "")) { | |
await locationStore.refreshCurrentLocation(); | |
} | |
try { | |
if (LOCATION_PAGES.has(route.name?.toString() || "")) { | |
await locationStore.refreshLocations(); | |
} | |
if (CURRENT_LOCATION_PAGES.has(route.name?.toString() || "")) { | |
await locationStore.refreshCurrentLocation(); | |
} | |
} catch (error) { | |
console.error('Failed to refresh location data:', error); | |
// Let the page render anyway – error states will be handled by components | |
} |
🤖 Prompt for AI Agents
In pages/dashboard.vue around lines 11 to 17, the top-level await calls to
locationStore.refreshLocations() and refreshCurrentLocation() lack error
handling; wrap each data refresh in try/catch (or attach .catch()) to prevent a
thrown error from stopping page render, log the error (or send to your existing
logger/telemetry) and surface a user-friendly fallback (e.g., set an error state
or skip blocking render), or run them non-blocking in the background with
Promise.allSettled if they can be concurrent.
sidebarStore.SidebarTopItems.push({ | ||
id: "link-dashboard", | ||
label: currentLocation.value ? currentLocation.value.name : "View Logs", | ||
label: currentLocation.value.name, | ||
icon: "tabler:map", | ||
to: { | ||
name: "dashboard-location-slug", | ||
params: { | ||
slug: currentLocation.value?.slug, | ||
slug: route.params.slug, | ||
}, | ||
}, | ||
}, | ||
{ | ||
}); |
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.
Fix duplicate sidebar item ID.
The sidebar item for the current location has the same ID "link-dashboard" as the "Back to Locations" item (line 41). This violates the uniqueness requirement for IDs.
- id: "link-dashboard",
+ id: "link-current-location",
📝 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.
sidebarStore.SidebarTopItems.push({ | |
id: "link-dashboard", | |
label: currentLocation.value ? currentLocation.value.name : "View Logs", | |
label: currentLocation.value.name, | |
icon: "tabler:map", | |
to: { | |
name: "dashboard-location-slug", | |
params: { | |
slug: currentLocation.value?.slug, | |
slug: route.params.slug, | |
}, | |
}, | |
}, | |
{ | |
}); | |
sidebarStore.SidebarTopItems.push({ | |
id: "link-current-location", | |
label: currentLocation.value.name, | |
icon: "tabler:map", | |
to: { | |
name: "dashboard-location-slug", | |
params: { | |
slug: route.params.slug, | |
}, | |
}, | |
}); |
🤖 Prompt for AI Agents
In pages/dashboard.vue around lines 49 to 59, the new sidebar item uses the same
id "link-dashboard" as another item (line 41) causing duplicate IDs; change the
id to a unique value (e.g., include the location slug or a distinct suffix like
`link-dashboard-{slug}` or `link-dashboard-current`) so IDs remain unique, for
example build the id from route.params.slug or a fixed distinct string before
pushing the item.
async function onSubmit(values: InsertLocation) { | ||
await $fetch("/api/location", { | ||
method: "POST", | ||
body: values, | ||
}); |
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.
💡 Verification agent
🧩 Analysis chain
Incorrect endpoint path and missing CSRF protection on create
- Endpoint should be POST /api/locations (plural), per the server route and other code paths.
- Use the same CSRF-protected fetch used elsewhere to be consistent with auth/CSRF middleware.
Apply this diff to the submit handler:
-async function onSubmit(values: InsertLocation) {
- await $fetch("/api/location", {
- method: "POST",
- body: values,
- });
-}
+async function onSubmit(values: InsertLocation) {
+ await $csrfFetch("/api/locations", {
+ method: "POST",
+ body: values,
+ });
+}
And add (outside this hunk) at the top of the script:
const { $csrfFetch } = useNuxtApp();
Run this to find other incorrect singular endpoints:
🏁 Script executed:
#!/bin/bash
# Find accidental uses of "/api/location" (missing 's').
rg -n --fixed-strings '/api/location' -S
Length of output: 369
Use the plural /api/locations
endpoint with CSRF protection in add.vue
Only pages/dashboard/add.vue is still using the incorrect singular endpoint and unprotected $fetch
. Update it as follows:
• File: pages/dashboard/add.vue
Replace the submit handler to call the plural route via $csrfFetch
and import the helper.
<script setup lang="ts">
+ const { $csrfFetch } = useNuxtApp();
- async function onSubmit(values: InsertLocation) {
- await $fetch("/api/location", {
- method: "POST",
- body: values,
- });
- }
+ async function onSubmit(values: InsertLocation) {
+ await $csrfFetch("/api/locations", {
+ method: "POST",
+ body: values,
+ });
+ }
</script>
All other references (stores/locations.ts
, the edit page) already use /api/locations
and $csrfFetch
.
📝 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.
async function onSubmit(values: InsertLocation) { | |
await $fetch("/api/location", { | |
method: "POST", | |
body: values, | |
}); | |
<script setup lang="ts"> | |
const { $csrfFetch } = useNuxtApp(); | |
async function onSubmit(values: InsertLocation) { | |
await $csrfFetch("/api/locations", { | |
method: "POST", | |
body: values, | |
}); | |
} | |
</script> |
🤖 Prompt for AI Agents
In pages/dashboard/add.vue around lines 4 to 8, the submit handler currently
uses the singular "/api/location" and unprotected $fetch; update the handler to
call the plural endpoint "/api/locations" using the CSRF-protected helper
($csrfFetch) and import that helper at the top of the file. Replace the await
$fetch(...) call with await $csrfFetch("/api/locations", { method: "POST", body:
values }) and add the appropriate import for $csrfFetch from your csrf helper
module so the form submission uses the same protected endpoint as the rest of
the app.
<LocationForm | ||
:on-submit | ||
submit-label="Add" | ||
submit-icon="tabler:circle-plus-filled" | ||
:on-submit-complete | ||
/> |
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.
Broken prop bindings: pass actual functions to LocationForm props
Same issue as edit.vue: you need to bind the functions, not bare boolean props.
Apply this diff:
- <LocationForm
- :on-submit
- submit-label="Add"
- submit-icon="tabler:circle-plus-filled"
- :on-submit-complete
- />
+ <LocationForm
+ :on-submit="onSubmit"
+ submit-label="Add"
+ submit-icon="tabler:circle-plus-filled"
+ :on-submit-complete="onSubmitComplete"
+ />
📝 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.
<LocationForm | |
:on-submit | |
submit-label="Add" | |
submit-icon="tabler:circle-plus-filled" | |
:on-submit-complete | |
/> | |
<LocationForm | |
:on-submit="onSubmit" | |
submit-label="Add" | |
submit-icon="tabler:circle-plus-filled" | |
:on-submit-complete="onSubmitComplete" | |
/> |
🤖 Prompt for AI Agents
In pages/dashboard/add.vue around lines 26 to 31 the LocationForm props
:on-submit and :on-submit-complete are bound without values (they become
booleans) — replace those bare bindings with references to the actual handler
functions (for example :on-submit="handleSubmit" and
:on-submit-complete="handleSubmitComplete"), ensure those methods are defined in
the component (or imported) and correctly bound to the component instance, and
keep the submit-label and submit-icon props as-is.
<LocationForm | ||
v-if="locationStore.currentLocationStatus !== 'pending'" | ||
:initial-values="locationStore.currentLocation" | ||
:on-submit | ||
submit-label="Update" | ||
submit-icon="tabler:map-pin-up" | ||
:on-submit-complete | ||
/> |
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.
Broken prop bindings: pass actual functions to LocationForm props
:on-submit
and :on-submit-complete
are missing bound values. As written, they’ll evaluate to boolean true
, and LocationForm will attempt to call a non-function, causing a runtime error.
Apply this diff:
- <LocationForm
- v-if="locationStore.currentLocationStatus !== 'pending'"
- :initial-values="locationStore.currentLocation"
- :on-submit
- submit-label="Update"
- submit-icon="tabler:map-pin-up"
- :on-submit-complete
- />
+ <LocationForm
+ v-if="locationStore.currentLocationStatus !== 'pending'"
+ :initial-values="locationStore.currentLocation"
+ :on-submit="onSubmit"
+ submit-label="Update"
+ submit-icon="tabler:map-pin-up"
+ :on-submit-complete="onSubmitComplete"
+ />
📝 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.
<LocationForm | |
v-if="locationStore.currentLocationStatus !== 'pending'" | |
:initial-values="locationStore.currentLocation" | |
:on-submit | |
submit-label="Update" | |
submit-icon="tabler:map-pin-up" | |
:on-submit-complete | |
/> | |
<LocationForm | |
v-if="locationStore.currentLocationStatus !== 'pending'" | |
:initial-values="locationStore.currentLocation" | |
:on-submit="onSubmit" | |
submit-label="Update" | |
submit-icon="tabler:map-pin-up" | |
:on-submit-complete="onSubmitComplete" | |
/> |
🤖 Prompt for AI Agents
In pages/dashboard/location/[slug]/edit.vue around lines 24 to 31, the template
uses :on-submit and :on-submit-complete without values so they become boolean
true; replace those broken bindings with actual function references (e.g.
:on-submit="handleSubmit" and :on-submit-complete="handleSubmitComplete" or
inline handlers like :on-submit="(vals)=>saveLocation(vals)" ) and implement or
wire the corresponding methods in the component (methods or setup functions) to
accept the payload and handle success/completion.
if (existingLocation && existingLocation.slug !== slug) { | ||
return sendError(event, createError({ | ||
statusCode: 409, | ||
statusMessage: "A location with that name already exists. Please choose a different name.", | ||
})); | ||
} | ||
|
||
return updateLocationBySlug(result.data, slug, event.context.user.id); | ||
}); |
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.
🛠️ Refactor suggestion
Return 404 when the slug doesn’t exist for the user
If no location matches the slug/user, updateLocationBySlug
will likely return undefined
. Returning that as-is produces a 200 with empty body. Prefer an explicit 404.
Suggested change:
- return updateLocationBySlug(result.data, slug, event.context.user.id);
+ const updated = await updateLocationBySlug(result.data, slug, event.context.user.id);
+ if (!updated) {
+ return sendError(event, createError({
+ statusCode: 404,
+ statusMessage: "Location not found.",
+ }));
+ }
+ return updated;
📝 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.
if (existingLocation && existingLocation.slug !== slug) { | |
return sendError(event, createError({ | |
statusCode: 409, | |
statusMessage: "A location with that name already exists. Please choose a different name.", | |
})); | |
} | |
return updateLocationBySlug(result.data, slug, event.context.user.id); | |
}); | |
if (existingLocation && existingLocation.slug !== slug) { | |
return sendError(event, createError({ | |
statusCode: 409, | |
statusMessage: "A location with that name already exists. Please choose a different name.", | |
})); | |
} | |
const updated = await updateLocationBySlug(result.data, slug, event.context.user.id); | |
if (!updated) { | |
return sendError(event, createError({ | |
statusCode: 404, | |
statusMessage: "Location not found.", | |
})); | |
} | |
return updated; | |
}); |
🤖 Prompt for AI Agents
In server/api/locations/[slug].put.ts around lines 16 to 24, the handler
currently returns the result of updateLocationBySlug directly which can be
undefined when no location matches the slug for the user, causing a 200 with
empty body; modify the code to check the return value of updateLocationBySlug
and if it is undefined call sendError(event, createError({ statusCode: 404,
statusMessage: "Location not found" })) so the endpoint returns an explicit 404,
otherwise return the updated location as before.
refactor: update location handling and related components
feat(api): add location update endpoint
style: clean up old form implementation
build: update dependencies and imports
closes #34
Summary by CodeRabbit
New Features
Improvements