-
Notifications
You must be signed in to change notification settings - Fork 1
first commit for create issue Template UI #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds template-management UI and supporting components: a CreateIssueTemplateDialog wizard, IssueTemplates view and routes, a reusable ExpandableCard, SvgWrapper and centralized icon catalog, plus sidebar layout/style tweaks and the @mdi/js dependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Root as Root.vue
participant EB as EventBus
participant Dialog as CreateIssueTemplateDialog
participant API as GraphQL
User->>Root: Click "Templates" / "Create Issue Template"
Root->>EB: emit(create-issue-template)
EB-->>Dialog: open & reset
User->>Dialog: navigate wizard & edit items
Dialog->>API: submit final template
API-->>Dialog: response
Dialog-->>EB: close
sequenceDiagram
autonumber
actor User
participant Card as ExpandableCard
User->>Card: Click pencil (collapsed)
Card-->>User: emit(expand)
User->>Card: Edit name/description
User->>Card: Click Confirm
Card-->>User: emit(confirm {name, description})
User->>Card: Click Cancel
Card-->>User: emit(cancel)
User->>Card: Click Delete
Card-->>User: emit(delete)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Potential focus areas:
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
🧹 Nitpick comments (19)
src/components/SideBarButton.vue (3)
3-3: Emit the declared click event (and keep navigation).Currently defineEmits is unused; click isn’t emitted. Emit it and preserve router navigation.
- <button class="sidebar-button" :class="{ active: isActive }" @click="navigate"> + <button + type="button" + class="sidebar-button" + :class="{ active: isActive }" + @click="(e) => { emit('click', e); navigate(e); }" + >-defineEmits(["click"]); +const emit = defineEmits<{ + (e: "click", ev: MouseEvent): void; +}>();Also applies to: 51-51
4-4: Avoid double centering (utility classes vs. CSS).You’re centering via classes and scoped CSS; keep one to reduce redundancy.
Option A (keep utilities): remove CSS rules
.icon-container { - display: flex; - align-items: center; - justify-content: center; overflow: hidden;Option B (keep CSS): drop the utility classes on the container
- <div class="icon-container d-flex align-center justify-center"> + <div class="icon-container">Also applies to: 71-74
3-3: Use type="button" to prevent unintended form submits.Small safety for usage inside forms.
- <button class="sidebar-button" :class="{ active: isActive }" @click="navigate"> + <button type="button" class="sidebar-button" :class="{ active: isActive }" @click="navigate">src/components/SideBar.vue (2)
7-7: Prefer stable keys over index to avoid unnecessary re-renders.- <div v-for="(item, index) in itemGroup" :key="index" class="sidebar-item mb-2 d-flex align-center justify-center"> + <div + v-for="(item, index) in itemGroup" + :key="('name' in item) ? `icon-${item.name}` : `fab-${item.icon}`" + class="sidebar-item mb-2 d-flex align-center justify-center" + >
17-17: Guard optional color to avoid “undefined-container”.When item.color is missing, default to a theme color.
- :color="`${item.color}-container`" + :color="`${(item.color ?? 'secondary')}-container`"src/assets/icons.ts (1)
2-2: Add typing for iconList for safer consumption.-export const iconList = [ +export interface IconDef { name: string; iconPath: string } +export const iconList: ReadonlyArray<IconDef> = [src/components/SvgIcon.vue (1)
3-13: Optional a11y: accept aria-label and role when needed.If used non-decoratively, allow overriding aria-hidden and role.
- <svg + <svg xmlns="http://www.w3.org/2000/svg" :viewBox="viewBox" fill="currentColor" class="svg-icon" :width="size" :height="size" - aria-hidden="true" + aria-hidden="true" + role="img" >src/components/ExpandableCard.vue (2)
58-61: Trim inputs before emitting confirm.Prevents accidental leading/trailing spaces in stored values.
- @click="emit('confirm', { name: localName, description: localDescription })" + @click="emit('confirm', { name: localName.trim(), description: localDescription.trim() })"
99-108: Keep local fields in sync when props change while expanded.watch( () => props.expandedCardKey, () => { // reset fields when expanded if (isExpanded.value) { localName.value = props.name; localDescription.value = props.description; } } ); + +watch( + () => [props.name, props.description], + () => { + if (isExpanded.value) { + localName.value = props.name; + localDescription.value = props.description; + } + } +);src/router/index.ts (2)
36-67: Avoid repeating component logic; drive variants via meta.All children render the same component. Add a meta templateType (issue|artefact|component|interface-specification|relation) so the view can switch behavior without route-name switches.
- { - path: "component", - name: "templates-component", - component: () => import("../views/home/Templates.vue"), - }, + { + path: "component", + name: "templates-component", + component: () => import("../views/home/Templates.vue"), + meta: { templateType: "component" as const }, + },(Apply similarly to the other children.)
36-39: Drop parent route name to prevent accidental navigation.The parent “templates” named route is unused and can be confusing when navigating by name. Remove name or add a redirect to the default child.
- { - path: "templates", - name: "templates", - component: () => import("../views/RouterOnly.vue"), + { + path: "templates", + // name: "templates", + component: () => import("../views/RouterOnly.vue"), children: [src/views/home/Templates.vue (1)
23-25: Remove non-English TODO comment from template.Avoid shipping internal notes in UI-facing markup.
- <!-- muss definitiv noch überabeitet werden und an die template Typen angepasst werde. das ne theo sesh -->src/components/dialog/CreateTemplateDialog.vue (3)
67-69: Emit payload type/name mismatch.Rename emitted argument to template for clarity.
-const emit = defineEmits<{ - (event: "created-template", project: IdObject): void; -}>(); +const emit = defineEmits<{ + (event: "created-template", template: IdObject): void; +}>();
71-78: Tighten validation.Constrain templateType to allowed values and validate repositoryURL as URL.
- yup.object().shape({ + yup.object().shape({ name: yup.string().required().label("Name"), - templateType: yup.string().required().label("Template type"), + templateType: yup + .mixed<"Artefact"|"Component"|"InterfaceSpecification"|"Issue"|"Relation">() + .oneOf(["Artefact","Component","InterfaceSpecification","Issue","Relation"]) + .required() + .label("Template type"), description: yup.string().notRequired().label("Description"), - repositoryURL: yup.string().notRequired().label("Repository URL") + repositoryURL: yup.string().url().notRequired().label("Repository URL") })
25-35: Avoid empty color prop.Remove color="" to inherit default text color.
-<DefaultButton variant="text" color="" @click="!isDirty && cancelCreateTemplate()"> +<DefaultButton variant="text" @click="!isDirty && cancelCreateTemplate()">src/components/dialog/CreateIssueTemplateDialog.vue (4)
255-257: Typo: “Telation”.Fix label/comment typo.
- <!-- Step 3: Assignment & Telation Types --> + <!-- Step 3: Assignment & Relation Types -->
632-633: Remove debug logging.Leftover console.log in production code.
- console.log(issuePriorities.value)
132-134: Translate/remove German warning in UI.Keep user-facing text in English and actionable.
- ⚠️ Path möglicherweise außerhalb von 100x100 ViewBox + ⚠️ Path may exceed the 100x100 viewBox; ensure it fits.
767-771: Unused field definitions.issueTypeName fields are unused; remove to reduce noise.
-const [issueTypeName, issueTypeNameProps ] = defineField('issueTypeName', fieldConfig);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
src/assets/icons.ts(1 hunks)src/components/ExpandableCard.vue(1 hunks)src/components/SideBar.vue(1 hunks)src/components/SideBarButton.vue(1 hunks)src/components/SvgIcon.vue(1 hunks)src/components/dialog/CreateIssueTemplateDialog.vue(1 hunks)src/components/dialog/CreateTemplateDialog.vue(1 hunks)src/router/index.ts(1 hunks)src/views/home/Root.vue(3 hunks)src/views/home/Templates.vue(1 hunks)
🔇 Additional comments (7)
src/components/ExpandableCard.vue (1)
21-26: Confirm IconButton availability (global or import).IconButton isn’t imported locally; ensure it’s globally registered to avoid runtime errors.
src/views/home/Root.vue (3)
33-35: Tabs addition LGTM.Templates tab placement and path look consistent with routes.
69-98: Right-bar actions likely unhandled for non-issue types.Only CreateIssueTemplateDialog is mounted in Templates.vue, and CreateTemplateDialog listens to create-issue-template. The four other events (artefact/component/interface-specification/relation) won’t open anything.
Would you like me to wire CreateTemplateDialog to these events and mount it in Templates.vue?
138-175: Verify custom icon tokens exist.Confirm "$issue", "$component", "$interface" are registered; otherwise Vuetify will render blanks. If not, point them to SvgIcon or mdi equivalents.
src/views/home/Templates.vue (2)
41-45: Confirm sort field typing.Template maps to an array of fields; ensure PaginatedList accepts Field | Field[]; otherwise split into separate named sorts.
71-82: Route helper still navigates to component detail.If this page will manage templates, consider routing to a template detail instead (or annotate as intentional placeholder).
src/components/dialog/CreateIssueTemplateDialog.vue (1)
366-367: Combobox model is array of objects; define item-title/value or map to strings.Otherwise chips will display “[object Object]”.
-<v-combobox v-model="issueStates" label="Issue States" multiple chips clearable class="mb-2" /> +<v-combobox + v-model="issueStates" + :items="issueStates" + item-title="name" + item-value="name" + label="Issue States" multiple chips clearable class="mb-2" />(Or maintain a separate string[] model.)
nk-coding
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls also run the formatter (npm run format)
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: 0
♻️ Duplicate comments (6)
src/components/dialog/CreateIssueTemplateDialog.vue (4)
575-582: Validate SVG path before adding to iconList.The
confirmAddIconfunction accepts arbitrary SVG path data without validation, which could lead to malformed or malicious SVG being rendered.Apply this diff to add validation:
function confirmAddIcon() { - if (newIcon.value.name && newIcon.value.iconPath) { + const allowed = /^[MmLlHhVvCcSsQqTtAaZz0-9 ,.\-]*$/; + const cleanedPath = newIcon.value.iconPath.trim().replace(/"/g, ""); + if (newIcon.value.name && cleanedPath && allowed.test(cleanedPath)) { - iconList.value.push({ name: newIcon.value.name, iconPath: newIcon.value.iconPath.trim().replace(/\"/g, "") }); - selectedIcon.value = { name: newIcon.value.name, iconPath: newIcon.value.iconPath.trim().replace(/\"/g, "") }; + iconList.value.push({ name: newIcon.value.name, iconPath: cleanedPath }); + selectedIcon.value = { name: newIcon.value.name, iconPath: cleanedPath }; newIcon.value = { name: "", iconPath: "" }; activeTab.value = "select"; } }
503-506: expandedCardKey type is incomplete.The type union only allows
"type" | "priority" | "state"but the code sets it to"assignment"(line 329) and"relation"(line 379) elsewhere.Apply this diff:
const expandedCardKey = ref<{ nameID: string; - type: "type" | "priority" | "state"; + type: "type" | "priority" | "state" | "assignment" | "relation"; } | null>(null);
719-731: Duplicate-name check uses wrong collection.The duplicate check compares
newNameagainstissueStatesinstead ofassignmentTypes.Apply this diff:
if (previousName.trim().toLowerCase() !== newName.trim().toLowerCase()) { - if (issueStates.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { + if (assignmentTypes.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { nameErrorMessage.value = "Name already exists"; return; } else { nameErrorMessage.value = ""; } }
746-758: Duplicate-name check uses wrong collection.The duplicate check compares
newNameagainstissueStatesinstead ofrelationTypes.Apply this diff:
if (previousName.trim().toLowerCase() !== newName.trim().toLowerCase()) { - if (issueStates.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { + if (relationTypes.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { nameErrorMessage.value = "Name already exists"; return; } else { nameErrorMessage.value = ""; } }src/views/home/Templates.vue (2)
23-24: Broken event handler: missing selectTemplate().The dialog emits
created-templatebut the handler calls an undefinedselectTemplate. Use the existingselectComponentor implementselectTemplate.Apply this diff to use the existing handler:
- <CreateIssueTemplateDialog @created-template="(template: IdObject) => selectTemplate(template)" /> + <CreateIssueTemplateDialog @created-template="(template: IdObject) => selectComponent(template)" />
53-66: Filtering and pagination are incorrect.The current logic treats an empty string as a valid filter and skips pagination for filtered results. Trim the filter to decide the path, and pass
skipfor filtered queries.Apply this diff to fix the filtering logic:
- if (filter == undefined) { + const q = (filter ?? "").trim(); + if (!q) { const res = await client.getComponentList({ orderBy, count, skip: page * count }); return [res.components.nodes, res.components.totalCount]; } else { const res = await client.getFilteredComponentList({ - query: filter, - count + query: q, + count, + skip: page * count, + orderBy }); - return [res.searchComponents, res.searchComponents.length]; + return [res.searchComponents.nodes ?? res.searchComponents, res.searchComponents.totalCount ?? res.searchComponents.length]; }(Adjust field access to match your GraphQL schema.)
🧹 Nitpick comments (1)
src/components/SvgWrapper.vue (1)
12-12: Validate SVG path data to prevent malformed or malicious paths.The current normalization only trims whitespace and removes quotes, which doesn't protect against malformed or potentially malicious SVG path commands. Consider adding a lightweight allowlist regex to validate that the path contains only valid SVG path syntax.
Apply this diff to add validation:
-const normalizedPath = computed(() => props.path.trim().replace(/"/g, "")); +const normalizedPath = computed(() => { + const cleaned = props.path.trim().replace(/"/g, ""); + const allowed = /^[MmLlHhVvCcSsQqTtAaZz0-9 ,.\-]*$/; + return allowed.test(cleaned) ? cleaned : ""; +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/components/ExpandableCard.vue(1 hunks)src/components/SideBar.vue(1 hunks)src/components/SideBarButton.vue(1 hunks)src/components/SvgWrapper.vue(1 hunks)src/components/dialog/CreateIssueTemplateDialog.vue(1 hunks)src/components/icons.ts(1 hunks)src/router/index.ts(1 hunks)src/views/home/Root.vue(3 hunks)src/views/home/Templates.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/SideBarButton.vue
- src/components/SideBar.vue
- src/router/index.ts
🔇 Additional comments (3)
src/views/home/Root.vue (1)
33-34: LGTM!The Templates navigation integration follows the existing pattern for projects, components, and IMSs. The permission checks, event bus wiring, and sidebar items are consistent with the established codebase conventions.
Also applies to: 69-98, 138-173
src/components/icons.ts (1)
118-278: LGTM! Icon catalog follows best practices.The use of
@mdi/jsfor Material Design icons is the recommended approach for tree-shakeable SVG paths, as noted in the learnings. The custom Gropius icons are included with inline SVG path data, which provides a centralized icon asset catalog for the UI.Based on learnings
src/components/ExpandableCard.vue (1)
1-97: LGTM!The ExpandableCard component is well-designed with proper state management. The computed
isExpandedprevents multiple cards from expanding simultaneously, and the watcher correctly resets local fields when a card is expanded. The component follows Vue 3 composition API best practices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/components/dialog/CreateIssueTemplateDialog.vue (3)
538-541: Expanded-card type union misses valid valuesLine 335 and below assign
"assignment"/"relation"toexpandedCardKey.type, but the union on Lines 538-541 only allows"type" | "priority" | "state", so TypeScript rejects those writes. Extend the union to cover every runtime value.const expandedCardKey = ref<{ nameID: string; - type: "type" | "priority" | "state"; + type: "type" | "priority" | "state" | "assignment" | "relation"; } | null>(null);
760-764: Assignment duplicate check hits the wrong collectionLine 760 still checks
issueStatesfor name collisions, so duplicate assignment types slip through. Compare againstassignmentTypesinstead before pushing.- if (issueStates.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { + if (assignmentTypes.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) {
787-791: Relation duplicate guard points to issue statesSame problem here: Lines 787-791 consult
issueStates, so duplicate relation type names aren’t caught. Guard againstrelationTypesinstead.- if (issueStates.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { + if (relationTypes.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/dialog/CreateIssueTemplateDialog.vue(1 hunks)src/components/icons.ts(1 hunks)src/router/index.ts(2 hunks)src/views/home/Root.vue(3 hunks)src/views/home/Templates/IssueTemplates.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/router/index.ts
- src/views/home/Root.vue
nk-coding
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LLemonLauncher there are still a lot of AI comments left, pls check them and resolve the ones which you say don't make sense (ideally with a comment) and fix the others
also there is 1 old comment from me remaining
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
62-62: Likely invalid semver for @types/react."^19.1.01" will not resolve as intended. Use "^19.1.1".
Apply this diff:
- "@types/react": "^19.1.01", + "@types/react": "^19.1.1",
🧹 Nitpick comments (5)
package.json (2)
38-38: Duplicate Prettier entry (deps vs devDeps).Prettier should be a devDependency only. Remove it from dependencies to avoid bloating prod installs.
Apply this diff:
"dependencies": { ... - "prettier": "^3.6.2", "react": "^19.1.1", ... }, "devDependencies": { ... "prettier": "^3.6.2",Also applies to: 67-67
19-21: Remove @mdi/font dependency
No MDI font CSS classes are referenced—icons use inline SVG via @mdi/js—so@mdi/fontcan be dropped from package.json to slim the bundle.src/components/icons.ts (3)
128-313: Add lightweight types, readonly, and a name→path map for O(1) lookups.Helps DX and prevents accidental mutation without changing current usage.
Apply this diff near the iconList declaration:
-export const iconList = [ +export interface IconSpec { name: string; iconPath: string } +export const iconList: ReadonlyArray<IconSpec> = [ { /* ...existing items... */ } -]; +]; + +export type IconName = (typeof iconList)[number]['name']; +export const iconMap: ReadonlyMap<IconName, string> = + new Map(iconList.map(i => [i.name as IconName, i.iconPath])); +export const getIconPath = (name: IconName) => iconMap.get(name);
193-313: Normalize naming style for icon “name”.Names mix PascalCase, kebab-case, and words with hyphens (e.g., "Chart-bar", "Component-Version", "GropiusBug"). Pick one convention (suggest kebab-case ids) and, if needed, add a separate human‑readable label.
Example approach:
type IconId = 'gropius-bug' | 'feature' | 'task' | ...; interface IconSpec { id: IconId; label: string; iconPath: string }If changing would be noisy now, add a TODO and follow up.
130-153: Optional: extract very long custom SVG paths into named constants.Improves readability and reduces diff churn in future edits.
Example:
const PATH_GROPIUS_BUG = "M12 3.75C14.071 ... 12 5.25Z"; const PATH_ISSUE = "M 12 3.255 C 14.1954 ... 12 4.845 Z"; export const iconList = [ { name: "GropiusBug", iconPath: PATH_GROPIUS_BUG }, { name: "Issue", iconPath: PATH_ISSUE }, // ... ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.json(1 hunks)src/components/ExpandableCard.vue(1 hunks)src/components/dialog/CreateIssueTemplateDialog.vue(1 hunks)src/components/icons.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/dialog/CreateIssueTemplateDialog.vue
- src/components/ExpandableCard.vue
🔇 Additional comments (2)
package.json (1)
20-20: Add of @mdi/js looks good.Required for the new icon catalog in src/components/icons.ts. No concerns.
src/components/icons.ts (1)
1-126: Import block is fine; tree‑shaken named imports.Given Vite + ESM, only referenced icon constants should be included. No action required.
If bundle size matters, consider lazy-loading large icon sets per route. Verify via bundle analyzer before acting.
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: 3
🧹 Nitpick comments (3)
src/components/icons.ts (2)
1-125: Consider type annotation for better IntelliSense.The exported
iconListlacks an explicit type, which limits editor autocompletion and type safety. Add a type annotation or useas constto enable better type inference.Apply this diff to add type safety:
+type IconEntry = { + readonly name: string; + readonly iconPath: string; +}; + -export const iconList = [ +export const iconList: readonly IconEntry[] = [Alternatively, use
as constfor literal type inference:-export const iconList = [ +export const iconList = [ // ... entries -]; +] as const;
128-189: Extract lengthy SVG paths to improve readability.The custom icon paths (especially
Gropius-Bug,Interface-Specification,Interface-Specification-Version, andInterface-Part) contain very long inline SVG path strings that reduce code readability and make maintenance difficult.Consider extracting these to named constants:
const GROPIUS_BUG_PATH = "M12 3.75C14.071 3.75..." // existing path export const iconList = [ { name: "Gropius-Bug", iconPath: GROPIUS_BUG_PATH }, // ... ];Or move complex custom icons to a separate file (e.g.,
customIcons.ts) and import them.src/components/dialog/CreateIssueTemplateDialog.vue (1)
609-624: Minor inconsistency in path validation.The validation regex (line 610) allows double quotes
"but then strips them duringconfirmAddIcon(lines 619-620). Consider removing quotes from the allowed pattern if they're not needed in the final path.-const originalAllowedPathElements = (path: string) => /^[MmLlHhVvCcSsQqTtAaZz0-9 ,."\-]+$/g.test(path); +const originalAllowedPathElements = (path: string) => /^[MmLlHhVvCcSsQqTtAaZz0-9 ,.\-]+$/g.test(path);Or accept quotes but don't strip them if validation already passed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/dialog/CreateIssueTemplateDialog.vue(1 hunks)src/components/icons.ts(1 hunks)src/router/index.ts(2 hunks)src/views/home/Root.vue(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/views/home/Root.vue
- src/router/index.ts
🔇 Additional comments (9)
src/components/icons.ts (1)
190-310: LGTM! Well-structured icon catalog.The standard icon mappings are clean, well-organized, and use consistent naming conventions. Leveraging @mdi/js is a solid choice for comprehensive icon coverage.
src/components/dialog/CreateIssueTemplateDialog.vue (8)
1-36: LGTM! Clean form structure with good validation feedback.The general information step properly uses vee-validate bindings and provides helpful inline warnings for duplicate template names.
97-193: Well-implemented icon management with proper security measures.The icon picker includes search, lazy loading for performance, and proper SVG path validation (line 610). The use of
SvgWrapperinstead ofv-htmladdresses the previous XSS concerns.
539-564: LGTM! Proper form validation setup.The vee-validate integration with yup schema is correctly implemented, and the async duplicate check on template name provides good UX feedback.
626-663: LGTM! Sensible defaults for issue template entities.The pre-populated defaults for issue types, priorities, states, and relation types provide a good starting point for users.
665-689: LGTM! Correct implementation with proper validation.The function properly validates names, checks for duplicates, and maintains sorted order.
695-727: LGTM! Proper numeric validation for priority values.The function correctly validates both the name and numeric value fields with appropriate error messages.
823-833: LGTM! Proper cleanup on cancel.The function correctly resets all error states and removes any uncommitted empty entries.
841-854: Placeholder submission needs implementation.Line 851 has a
console.logplaceholder instead of actual GraphQL mutation to create the template. Ensure this is implemented before the feature is complete.Do you want me to help generate the GraphQL mutation code for submitting the template, or is this tracked in a separate issue?
| @click=" | ||
| () => { | ||
| createIssueTypeInput('', '', '', ''); | ||
| expandedCardKey = { nameID: '', type: 'type' }; | ||
| } | ||
| " | ||
| >+ Add Issue Type | ||
| </v-btn> | ||
| <ExpandableCard | ||
| v-for="IssueTypeInput in IssueTypes" | ||
| :key="IssueTypeInput.name" | ||
| :name="IssueTypeInput.name" | ||
| :description="IssueTypeInput.description" | ||
| :expandedCardKey="expandedCardKey" | ||
| type="type" | ||
| :nameErrorMessage="nameErrorMessage" | ||
| @expand=" | ||
| () => { | ||
| expandedCardKey = { nameID: IssueTypeInput.name, type: 'type' }; | ||
| selectedIcon = | ||
| iconList.find((icon) => icon.iconPath === IssueTypeInput.iconPath) ?? | ||
| null; | ||
| currentEditedName = IssueTypeInput.name; | ||
| currentEditedDescription = IssueTypeInput.description; | ||
| nameErrorMessage = ''; | ||
| } | ||
| " | ||
| @cancel="cancelCreateCard()" | ||
| @delete="deleteIssueTypeInputByName(IssueTypeInput.name)" | ||
| @confirm=" | ||
| ({ name, description }) => { | ||
| if (!name) { | ||
| nameErrorMessage = 'Name is required'; | ||
| return; | ||
| } | ||
| createIssueTypeInput( | ||
| IssueTypeInput.name, | ||
| name, | ||
| description, | ||
| selectedIcon?.iconPath ?? '' | ||
| ); | ||
| } | ||
| " |
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 | 🟠 Major
Extract inline handlers to named functions.
The inline event handlers contain significant logic (10+ lines) that reduces template readability and makes testing difficult. This pattern repeats throughout the component.
Extract to named functions in the script section:
function handleAddIssueType() {
createIssueTypeInput('', '', '', '');
expandedCardKey.value = { nameID: '', type: 'type' };
}
function handleExpandIssueType(issueType: IssueTypeInput) {
expandedCardKey.value = { nameID: issueType.name, type: 'type' };
selectedIcon.value = iconList.value.find((icon) => icon.iconPath === issueType.iconPath) ?? null;
currentEditedName.value = issueType.name;
currentEditedDescription.value = issueType.description;
nameErrorMessage.value = '';
}
function handleConfirmIssueType(issueType: IssueTypeInput, { name, description }: { name: string; description: string }) {
if (!name) {
nameErrorMessage.value = 'Name is required';
return;
}
createIssueTypeInput(issueType.name, name, description, selectedIcon.value?.iconPath ?? '');
}Then use in template:
@click="handleAddIssueType"
@expand="handleExpandIssueType(IssueTypeInput)"
@confirm="(data) => handleConfirmIssueType(IssueTypeInput, data)"Apply the same pattern to priorities, states, assignments, and relations.
🤖 Prompt for AI Agents
In src/components/dialog/CreateIssueTemplateDialog.vue around lines 47 to 89,
the template contains large inline event handlers for add/expand/confirm that
reduce readability and testability; extract those handlers into named functions
in the script section (e.g., handleAddIssueType, handleExpandIssueType,
handleConfirmIssueType) that call the existing helpers (createIssueTypeInput,
deleteIssueTypeInputByName, etc.), update reactive refs (expandedCardKey,
selectedIcon, currentEditedName, currentEditedDescription, nameErrorMessage)
appropriately, and return early on validation errors; then replace the inline
handlers in the template with these named functions (and a small arrow wrapper
for passing arguments to confirm if needed). Apply the same extraction pattern
for priorities, states, assignments, and relations to keep templates concise and
consistent.
| function createAssignmentType(previousName: string, newName: string, description: string) { | ||
| if (newName.trim().length === 0 && previousName.trim().length !== 0) { | ||
| nameErrorMessage.value = "Name is required"; | ||
| return; | ||
| } | ||
| if (previousName.trim().toLowerCase() !== newName.trim().toLowerCase()) { | ||
| if (issueStates.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { | ||
| nameErrorMessage.value = "Name already exists"; | ||
| return; | ||
| } else { | ||
| nameErrorMessage.value = ""; | ||
| } | ||
| } | ||
| deleteAssignmentTypeByName(previousName); | ||
| assignmentTypes.value.push({ name: newName, description }); | ||
| assignmentTypes.value.sort((a, b) => a.name.localeCompare(b.name)); | ||
| expandedCardKey.value = null; | ||
| currentEditedName.value = ""; | ||
| currentEditedDescription.value = ""; | ||
| } |
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.
Critical: Duplicate check uses wrong collection.
Line 767 checks issueStates.value for duplicates but should check assignmentTypes.value. This allows duplicate assignment type names to be created.
Apply this fix:
if (previousName.trim().toLowerCase() !== newName.trim().toLowerCase()) {
- if (issueStates.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) {
+ if (assignmentTypes.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) {
nameErrorMessage.value = "Name already exists";
return;
} else {🤖 Prompt for AI Agents
In src/components/dialog/CreateIssueTemplateDialog.vue around lines 761 to 782,
the duplicate-name check incorrectly inspects issueStates.value instead of
assignmentTypes.value; update the conditional to check
assignmentTypes.value.some(...) using the same trimmed, lowercased comparison
against newName, so duplicate assignment type names are detected and the early
return with the "Name already exists" message works as intended.
| function createRelationType(previousName: string, newName: string, inverseName: string, description: string) { | ||
| if (newName.trim().length === 0 && previousName.trim().length !== 0) { | ||
| nameErrorMessage.value = "Name is required"; | ||
| return; | ||
| } | ||
| if (previousName.trim().toLowerCase() !== newName.trim().toLowerCase()) { | ||
| if (issueStates.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { | ||
| nameErrorMessage.value = "Name already exists"; | ||
| return; | ||
| } else { | ||
| nameErrorMessage.value = ""; | ||
| } | ||
| } | ||
| if (inverseName.trim().length === 0 && previousName.trim().length !== 0) { | ||
| inverseNameErrorMessage.value = "Inverse Name is required"; | ||
| return; | ||
| } else { | ||
| inverseNameErrorMessage.value = ""; | ||
| } | ||
| deleteRelationTypeByName(previousName); | ||
| relationTypes.value.push({ name: newName, inverseName, description }); | ||
| relationTypes.value.sort((a, b) => a.name.localeCompare(b.name)); | ||
| expandedCardKey.value = null; | ||
| currentEditedName.value = ""; | ||
| currentEditedDescription.value = ""; | ||
| currentEditedInverseName.value = ""; | ||
| } |
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.
Critical: Duplicate check uses wrong collection.
Line 794 checks issueStates.value for duplicates but should check relationTypes.value. This allows duplicate relation type names to be created.
Apply this fix:
if (previousName.trim().toLowerCase() !== newName.trim().toLowerCase()) {
- if (issueStates.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) {
+ if (relationTypes.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) {
nameErrorMessage.value = "Name already exists";
return;
} else {🤖 Prompt for AI Agents
In src/components/dialog/CreateIssueTemplateDialog.vue around lines 788 to 817,
the duplicate-name check incorrectly inspects issueStates.value instead of
relationTypes.value; update the duplicate validation so it checks
relationTypes.value for any item whose name (trimmed, case-insensitive) matches
newName.trim().toLowerCase(), set nameErrorMessage.value to "Name already
exists" and return when found, otherwise clear the error; keep the rest of the
flow (deleteRelationTypeByName, push new relation, sort, reset UI state)
unchanged.
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: 3
♻️ Duplicate comments (2)
src/components/dialog/CreateIssueTemplateDialog.vue (2)
790-819: Critical: Duplicate-name check uses wrong collection.Line 796 checks
issueStates.valuefor duplicate relation type names but should checkrelationTypes.value. This bug allows duplicate relation type names to slip through validation.Apply this fix:
if (previousName.trim().toLowerCase() !== newName.trim().toLowerCase()) { - if (issueStates.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { + if (relationTypes.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { nameErrorMessage.value = "Name already exists"; return; } else {
763-784: Critical: Duplicate-name check uses wrong collection.Line 769 checks
issueStates.valuefor duplicate assignment type names but should checkassignmentTypes.value. This bug allows duplicate assignment type names to slip through validation.Apply this fix:
if (previousName.trim().toLowerCase() !== newName.trim().toLowerCase()) { - if (issueStates.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { + if (assignmentTypes.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { nameErrorMessage.value = "Name already exists"; return; } else {
🧹 Nitpick comments (2)
src/components/dialog/CreateIssueTemplateDialog.vue (1)
47-89: Extract inline handlers to named functions.The inline event handler contains ~40 lines of logic spread across multiple callbacks. Extract these to named functions in the script section for better readability and testability. Based on learnings
Extract handlers like:
function handleAddIssueType() { createIssueTypeInput('', '', '', ''); expandedCardKey.value = { nameID: '', type: 'type' }; } function handleExpandIssueType(issueType: IssueTypeInput) { expandedCardKey.value = { nameID: issueType.name, type: 'type' }; selectedIcon.value = iconList.value.find((icon) => icon.iconPath === issueType.iconPath) ?? null; currentEditedName.value = issueType.name; currentEditedDescription.value = issueType.description; nameErrorMessage.value = ''; } function handleConfirmIssueType(issueType: IssueTypeInput, data: { name: string; description: string }) { if (!data.name) { nameErrorMessage.value = 'Name is required'; return; } createIssueTypeInput(issueType.name, data.name, data.description, selectedIcon.value?.iconPath ?? ''); }Then use in template:
@click="handleAddIssueType" @expand="handleExpandIssueType(IssueTypeInput)" @confirm="(data) => handleConfirmIssueType(IssueTypeInput, data)"Apply the same pattern to priorities (lines 198-243), states (lines 276-316), assignments (lines 342-382), and relations (lines 392-442).
src/views/home/Root.vue (1)
69-98: Consider extracting repeated template sidebar logic.Lines 69-98 contain five nearly identical case blocks that differ only in template type name. Consider extracting a helper to reduce duplication:
const templateTypes = { 'templates-issue': 'issue template', 'templates-artefact': 'artefact template', 'templates-component': 'component template', 'templates-interface-specification': 'interface specification template', 'templates-relation': 'relation template' } as const; // Then in the switch: if (route.name && route.name.toString().startsWith('templates-')) { const typeName = templateTypes[route.name as keyof typeof templateTypes]; if (typeName) { name = typeName; eventName = `create-${route.name.toString().substring(10)}-template` as keyof Events; disabled = !(store.user?.canCreateTemplates ?? false); break; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/dialog/CreateIssueTemplateDialog.vue(1 hunks)src/router/index.ts(2 hunks)src/views/home/Root.vue(3 hunks)
🔇 Additional comments (2)
src/views/home/Root.vue (1)
144-179: LGTM: Template sidebar navigation.The left sidebar items for templates are well-structured with appropriate icons and route targets. The conditional rendering based on route name is clean and consistent.
src/router/index.ts (1)
385-395: Unable to fully verify due to sandbox environment limitations.The initial script output confirmed that:
- No template-specific components exist in the codebase
component/Home.vuerenders generic component details (displayingcomponent.name,component.description,component.pinnedIssues,component.issues)However, subsequent verification attempts failed to access the router file, preventing me from determining:
- Whether this route path is intentionally named "/templates" while displaying components
- Whether this is part of a broader routing pattern where "templates" is a feature alias for components
- The full context and purpose of this route within the application
The naming inconsistency between the route path ("/templates") and the component being loaded (component detail view) warrants manual verification to confirm whether this is intended behavior, a copy-paste error, or a placeholder.
| const allowedPathElements = ref<boolean>(false); | ||
| const originalAllowedPathElements = (path: string) => /^[MmLlHhVvCcSsQqTtAaZz0-9 ,."\-]+$/g.test(path); | ||
| watch( | ||
| () => newIcon.value.iconPath, | ||
| (newPath) => (allowedPathElements.value = originalAllowedPathElements(newPath)) | ||
| ); |
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.
🧩 Analysis chain
Verify inclusion of quotes in SVG path regex.
Line 606 includes " (double quotes) in the allowed SVG path characters. SVG path data should not contain literal quote characters. Verify whether this is intentional or if it should be removed.
🏁 Script executed:
#!/bin/bash
# Check if any SVG paths in the icon list contain quote characters
echo "Checking icon list for quotes in path data..."
cat src/components/icons.ts | grep -E 'iconPath.*".*"' | head -20Length of output: 45
I'll verify the SVG path specification and examine the actual icon data more thoroughly.
Let me examine the actual icon data and context more thoroughly:
Remove double quotes from SVG path validation regex.
SVG path data follows an EBNF grammar that specifies valid characters as command letters, numbers, and whitespace/comma separators. Coordinates can be separated by whitespace, commas, or both. Double quotes are not defined as valid path data characters in the SVG specification. The regex on line 606 includes " in the character class, which should be removed to accurately validate against the SVG path specification.
const originalAllowedPathElements = (path: string) => /^[MmLlHhVvCcSsQqTtAaZz0-9 ,.\-]+$/g.test(path);
🤖 Prompt for AI Agents
In src/components/dialog/CreateIssueTemplateDialog.vue around lines 605 to 611,
the SVG path validation regex incorrectly allows double quotes; update the
character class to remove the double-quote character so it only permits SVG path
command letters, digits, whitespace/comma, dot and hyphen, and also remove the
unnecessary global flag (/g) when using RegExp.test to avoid lastIndex
side-effects; replace the regex with a pattern that reflects those allowed chars
and use it in originalAllowedPathElements.
| function confirmAddIcon() { | ||
| if (newIcon.value.name && newIcon.value.iconPath && allowedPathElements) { | ||
| const newEntry = { | ||
| name: newIcon.value.name, | ||
| iconPath: newIcon.value.iconPath.trim().replace(/\"/g, "") | ||
| }; | ||
| iconList.value.unshift(newEntry); | ||
| selectedIcon.value = newEntry; | ||
| iconSearch.value = ""; | ||
| newIcon.value = { name: "", iconPath: "" }; | ||
| activeTab.value = "select"; | ||
| } | ||
| } |
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 ref access in conditional.
Line 614 references allowedPathElements without .value, but it's a ref. This will always evaluate to truthy (the ref object itself), not the boolean value inside.
Apply this fix:
function confirmAddIcon() {
- if (newIcon.value.name && newIcon.value.iconPath && allowedPathElements) {
+ if (newIcon.value.name && newIcon.value.iconPath && allowedPathElements.value) {
const newEntry = {
name: newIcon.value.name,
iconPath: newIcon.value.iconPath.trim().replace(/\"/g, "")🤖 Prompt for AI Agents
In src/components/dialog/CreateIssueTemplateDialog.vue around lines 613 to 626,
the conditional currently checks the ref allowedPathElements directly (which is
always truthy); change the condition to read allowedPathElements.value so the
inner boolean is evaluated. Update the if statement to use
allowedPathElements.value (and keep the existing newIcon.value checks) so the
branch only runs when the ref's boolean value is true.
| { | ||
| path: "templates", | ||
| name: "templates", | ||
| component: () => import("../views/RouterOnly.vue"), | ||
| children: [ | ||
| { | ||
| path: "", | ||
| name: "templates-issue", | ||
| component: () => import("../views/home/Templates/IssueTemplates.vue") | ||
| }, | ||
| { | ||
| path: "component", | ||
| name: "templates-component", | ||
| component: () => import("../views/home/Templates/IssueTemplates.vue") | ||
| }, | ||
| { | ||
| path: "artefact", | ||
| name: "templates-artefact", | ||
| component: () => import("../views/home/Templates/IssueTemplates.vue") | ||
| }, | ||
| { | ||
| path: "interface-specification", | ||
| name: "templates-interface-specification", | ||
| component: () => import("../views/home/Templates/IssueTemplates.vue") | ||
| }, | ||
| { | ||
| path: "relation", | ||
| name: "templates-relation", | ||
| component: () => import("../views/home/Templates/IssueTemplates.vue") | ||
| } | ||
| ] | ||
| }, |
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.
🧩 Analysis chain
All template child routes load IssueTemplates.vue.
Lines 44, 49, 54, 59, 64 all load IssueTemplates.vue for different template types (issue, component, artefact, interface-specification, relation). Verify whether each template type should have its own view component, or if IssueTemplates.vue is designed to handle all types dynamically.
🏁 Script executed:
#!/bin/bash
# Check if there are separate template view components for each type
fd -e vue ".*Template.*\.vue" src/views/home/Templates/
# Check if IssueTemplates.vue handles multiple template types
rg -A5 -B5 "templates-component|templates-artefact|templates-relation" src/views/home/Templates/IssueTemplates.vueLength of output: 43
I need to inspect how IssueTemplates.vue handles the different template types dynamically.
---
🏁 Script executed:
```shell
#!/bin/bash
# Look at IssueTemplates.vue to see how it handles different template types
head -100 src/views/home/Templates/IssueTemplates.vue
# Also search for any route-related patterns in the file
rg -n "\$route|useRoute|route\.|name:" src/views/home/Templates/IssueTemplates.vue | head -20
Length of output: 2707
I found a critical issue. The component IssueTemplates.vue only navigates to name: "templates-issue" and fetches only issue templates. Let me verify if the other route names are actually implemented elsewhere.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for any references to the other template route names in the codebase
rg "templates-component|templates-artefact|templates-interface-specification|templates-relation" --type=ts --type=js --type=vue -n
# Check if there are other template-related components
fd -e vue ".*Template.*\.vue" src/views/home/
Length of output: 71
Let me search without the file type restriction:
---
🏁 Script executed:
```shell
#!/bin/bash
# Search all files for references to the other template route names
rg "templates-component|templates-artefact|templates-interface-specification|templates-relation" -n
Length of output: 951
Now I need to check how Root.vue uses these route names:
---
🏁 Script executed:
```shell
#!/bin/bash
# View the case statements and navigation logic in Root.vue
sed -n '70,100p' src/views/home/Root.vue
# View the navigation links section
sed -n '150,180p' src/views/home/Root.vue
Length of output: 2335
Each template route must use its own dedicated component or IssueTemplates.vue must handle all template types dynamically.
The router defines five template routes that all point to IssueTemplates.vue, but the component is hardcoded to handle only templates-issue. Root.vue has separate event names for each template type (create-artefact-template, create-component-template, etc.), indicating each should have distinct functionality. Currently, routes templates-component, templates-artefact, templates-interface-specification, and templates-relation will fail—they load a component that only fetches and displays issue templates.
Either: (1) Create separate view components for each template type (ComponentTemplates.vue, ArtefactTemplates.vue, etc.), or (2) Refactor IssueTemplates.vue to dynamically handle all template types using route parameters.
🤖 Prompt for AI Agents
In src/router/index.ts around lines 36 to 67 the five "templates" child routes
all point to IssueTemplates.vue but IssueTemplates currently only handles issue
templates; this will break component/artefact/interface-specification/relation
routes. Fix by either (A) creating dedicated view components (e.g.,
ComponentTemplates.vue, ArtefactTemplates.vue,
InterfaceSpecificationTemplates.vue, RelationTemplates.vue) and update each
route to import its corresponding component, or (B) refactor IssueTemplates.vue
to be generic: accept a route param or prop for templateType, switch fetch/event
names based on that param (map templateType to the correct API call and event
names like create-artefact-template, create-component-template, etc.), and
update the router to pass the templateType as a prop to the component. Ensure
the chosen approach implements the correct data fetching and emits the
corresponding root events for each template type.
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: 0
♻️ Duplicate comments (5)
src/components/dialog/CreateIssueTemplateDialog.vue (5)
172-176: Critical: Missing.valueon ref check.Line 175 checks
allowedPathElementsdirectly (the ref object) instead ofallowedPathElements.value, so the button will always be enabled even when the path is invalid.Apply this fix:
:disabled=" !newIcon.name || !newIcon.iconPath || - !allowedPathElements + !allowedPathElements.value "
605-611: Critical: Remove double quotes from SVG path validation regex.Line 606 includes
"in the allowed character class for SVG paths. Per the SVG specification, path data should only contain command letters (M, L, C, etc.), numbers, whitespace, commas, dots, and hyphens. Double quotes are not valid path data characters. Additionally, the/gflag is unnecessary with.test()and can cause unexpected behavior due tolastIndexside effects.Apply this fix:
-const originalAllowedPathElements = (path: string) => /^[MmLlHhVvCcSsQqTtAaZz0-9 ,."\-]+$/g.test(path); +const originalAllowedPathElements = (path: string) => /^[MmLlHhVvCcSsQqTtAaZz0-9 ,.\-]+$/.test(path);
613-626: Critical: Missing.valueon ref check (duplicate location).Line 614 has the same issue as line 175 - checking the ref object directly instead of its value.
Apply this fix:
- if (newIcon.value.name && newIcon.value.iconPath && allowedPathElements) { + if (newIcon.value.name && newIcon.value.iconPath && allowedPathElements.value) {
760-781: Critical: Duplicate check uses wrong collection.Line 766 checks
issueStates.valuefor duplicates when it should checkassignmentTypes.value. This allows duplicate assignment type names to be created, corrupting the data model.Apply this fix:
if (previousName.trim().toLowerCase() !== newName.trim().toLowerCase()) { - if (issueStates.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { + if (assignmentTypes.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { nameErrorMessage.value = "Name already exists"; return;
787-816: Critical: Duplicate check uses wrong collection.Line 793 checks
issueStates.valuefor duplicates when it should checkrelationTypes.value. This allows duplicate relation type names to be created, corrupting the data model.Apply this fix:
if (previousName.trim().toLowerCase() !== newName.trim().toLowerCase()) { - if (issueStates.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { + if (relationTypes.value.some((item) => item.name.trim().toLowerCase() === newName.trim().toLowerCase())) { nameErrorMessage.value = "Name already exists"; return;
🧹 Nitpick comments (1)
src/views/home/Templates/IssueTemplates.vue (1)
48-63: Consider using the orderBy and page parameters.The
fetchItemsmethod receivesorderByandpageparameters but doesn't use them. If sorting and pagination offset are needed, these should be passed to the GraphQL queries.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/dialog/CreateIssueTemplateDialog.vue(1 hunks)src/util/eventBus.ts(1 hunks)src/views/home/Templates/IssueTemplates.vue(1 hunks)
🔇 Additional comments (9)
src/util/eventBus.ts (1)
16-20: LGTM!The new template creation events are correctly typed and follow the established naming conventions. This extends the event bus contract appropriately for the new template management functionality.
src/views/home/Templates/IssueTemplates.vue (3)
1-26: LGTM!The template structure is well-organized with proper pagination, sorting, and event handling. The CreateIssueTemplateDialog integration correctly calls the
selectTemplatehandler to navigate upon creation.
38-46: LGTM!Good use of type inference from the GraphQL client return type, and the sort field configuration is appropriate for template listing.
65-88: LGTM!The navigation helpers and styling are correctly implemented. The route parameter uses "trackable" which aligns with the domain model where templates are trackable entities.
src/components/dialog/CreateIssueTemplateDialog.vue (5)
1-36: LGTM!The dialog structure and general information form are well-implemented with proper validation and user feedback for duplicate template names.
333-458: LGTM!The assignment and relation type sections follow a consistent pattern with proper validation for required fields like inverseName.
460-492: LGTM!Good UX implementation with dirty state tracking and confirmation dialog to prevent accidental data loss.
628-758: LGTM!The initial data setup and CRUD functions for issue types, priorities, and states are well-structured with consistent validation patterns and proper state management.
822-865: LGTM!The cleanup, event handling, and dialog lifecycle functions are correctly implemented with proper state resets and navigation logic.
Summary by CodeRabbit
New Features
Style