fix(add menu): change item menu and connect to page with correct query - #1066
fix(add menu): change item menu and connect to page with correct query#1066alireza013013 wants to merge 8 commits into
Conversation
… userBoardId in query parameters - reset form state and apply query defaults on mount and route change - correct label from "Subject" to "Classification" in paper edit form - add findByTitle prop and enhance item selection logic
…menu for send query for multimedia - get query in create multimedia page and set value
…sponsiveness and layout
…and spacing and remove extra style
|
@alireza013013 is attempting to deploy a commit to the GamaEdtech Team on Vercel. A member of the Team first needs to authorize it. |
…front into feat/add-menu-footer
sanaderi
left a comment
There was a problem hiding this comment.
Review
Good idea overall (deep-linking the Add menu into pre-filled create forms), but I'd like to see one design change plus a few correctness issues addressed before merge.
Suggested design change: use real classification/content_type IDs in the query, not title slugs
Right now the links use human-readable slugs (classification=Study-and-Revision-Guide), and paper/create.vue does .replaceAll('-', ' ') plus a new findByTitle matching mode on the shared gomboBox.vue to resolve that back to the real backend item.
The search page already establishes the convention of passing real IDs in the query (route.query.test_type, etc.) rather than title text — worth following that here too. I checked the actual endpoints and the IDs are stable and safe to hardcode:
test_type(classification) — same ID across different boards (verified against board6627and board7131):- Worksheet =
310, Predicted Paper =8344, Study and Revision Guide =6896, Topical =7180
- Worksheet =
content_type(multimedia) — this endpoint doesn't even take a board param, so it's globally fixed:- Video =
6022, PowerPoint =6024
- Video =
Switching AddOptionBottomMenu.vue to use these numeric IDs directly (e.g. ?board=7131&classification=310, ?contentType=6022) would let applyQueryDefaults just assign the ID straight to the field — no string-replace hack — and makes the new findByTitle prop/logic on gomboBox.vue unnecessary entirely, since the existing id-based matching already handles it. Smaller surface area on a shared component, and immune to the backend ever renaming a classification label (title-matching would silently clear the field if that ever happens).
Bugs / correctness
userBoardId in AddOptionBottomMenu.vue is computed once, not reactively
const { user } = useUser()
const defaultBoardId = 6627
const userBoardId = user.value?.board ? user.value?.board : defaultBoardIduseUser()'s user is a useState('user', () => null) populated asynchronously. If this menu mounts before that resolves, userBoardId locks to the fallback 6627 forever for that mount, even after the real user board loads. Should be computed(() => user.value?.board || defaultBoardId).
Inconsistent form-reset between the two create pages
paper/create.vue's watch(() => route.query, ...) calls resetFormState() before reapplying query defaults. multimedia/create.vue's equivalent watcher only calls applyQueryDefaults() — it never resets the rest of the form. Since both pages are the same route path (only the query string changes), Nuxt's default per-path page-keying does not remount the component for query-only changes, which is exactly why the explicit watch was added — but only paper's fully resets state. If a user reopens the Add menu while already on /user/multimedia/create and picks a different option, stale title/description/uploaded file from the previous selection will linger. Worth adding the same reset multimedia's form is currently missing.
gomboBox.vue's findByTitle re-sync hardcodes foundObj.id (moot if the ID-based query approach above is adopted, otherwise:)
if (props.findByTitle && foundObj && !isSameValue(foundObj.id, value)) {
emit('update:modelValue', String(foundObj.id))
}Every other lookup in this file correctly uses item[props.itemValue] (configurable, default 'id'). This spot hardcodes .id. No current caller overrides item-value, so harmless today, but inconsistent with the rest of the component.
Scope / risk note
modal/base.vue padding change (pa-6 → py-6 px-3 px-sm-6) is a shared component used by many unrelated modals across the app. Reasonable-looking change, but worth a quick visual pass on a few other modals beyond this PR's own scope, since it affects every common-modal-base consumer.
…front into feat/add-menu-footer
- Use stable backend IDs for paper classifications and multimedia content types in add-menu links. - Make user board selection reactive while the user state loads. - Remove title-based gombo-box matching from create forms. - Reset multimedia form state when query defaults change.
Pull Request Template
Description
Type of Change
Checklist