chore: demo page menu management #5619
Conversation
|
WalkthroughThis pull request introduces new API endpoints for menu management, including listing, name existence, and path existence checks with token verification. It also adds a detailed mock menu data structure and updates localization files for both English and Chinese. Additionally, the PR enhances type safety in the form API, refines table adapter data access, adds new routes, and creates comprehensive Vue components for managing system menus through grid views and forms. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant TokenVerifier
Client->>API: Send request (list, name/path check)
API->>TokenVerifier: verifyAccessToken(token)
TokenVerifier-->>API: Return user info / null
alt Valid Token
API->>Client: Success response with relevant data
else Invalid Token
API->>Client: Unauthorized response
end
sequenceDiagram
participant User
participant Grid
participant API
participant FormDrawer
User->>Grid: Trigger refresh (onRefresh)
Grid->>API: getMenuList()
API-->>Grid: Return menu data
User->>Grid: Click action (edit/create/append/delete)
Grid->>FormDrawer: Open form drawer with selected data
FormDrawer->>API: Submit form data (create/update menu)
API-->>FormDrawer: Return success
FormDrawer->>Grid: Trigger grid refresh
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
apps/backend-mock/api/system/menu/path-exists.ts (2)
5-15: Consider improving the path mapping implementation.The pathMap initialization and population look good, but consider adding a comment explaining the purpose of initializing with
'/': 0.
17-28: Improve readability of the path existence logic.The condition for determining if a path exists is complex and could be more readable.
Consider refactoring like this:
const { id, path } = getQuery(event); - return (path as string) in pathMap && - (!id || pathMap[path as string] !== String(id)) - ? useResponseSuccess(true) - : useResponseSuccess(false); + // Check if path exists and has a different ID (conflict) + const pathExists = (path as string) in pathMap; + const isDifferentId = !id || pathMap[path as string] !== String(id); + const hasConflict = pathExists && isDifferentId; + + return useResponseSuccess(hasConflict);apps/backend-mock/api/system/menu/name-exists.ts (1)
17-28: Secure API endpoint implementationThe endpoint properly verifies access tokens before processing requests and returns appropriate responses. The logic for checking name existence while accounting for ID exclusion is concise and effective.
One thing to note: This API returns
truewhen a name exists (is a duplicate) and should not be used, which might be counterintuitive at first glance. Consider documenting this behavior in comments to make the intention clearer.+ // Returns true if the name already exists (is a duplicate) and cannot be used + // except when it belongs to the same menu item (matching id) return (name as string) in namesMap && (!id || namesMap[name as string] !== String(id)) ? useResponseSuccess(true) : useResponseSuccess(false);playground/src/views/system/menu/data.ts (1)
93-95: Localize the “新增下级” text for consistency.
The text “新增下级” is hardcoded, which might not align with your i18n approach. Converting it into a localized string helps maintain consistency across all locales.Example fix:
- text: '新增下级', + text: $t('system.menu.appendSubmenu'),(Assuming you add a corresponding locale entry in your translation files.)
playground/src/views/system/menu/modules/form.vue (1)
50-67: Consider adding fallback/error handling for asynchronous validations.
Your Zod refinements callisMenuNameExistsandisMenuPathExistsover a network. If these requests fail, the refinement might break unexpectedly. Adding a catch or fallback mechanism can gracefully handle such failures.playground/src/views/system/menu/list.vue (2)
94-111: Consider adding success loading state handling in the delete operationWhile the error case calls
hideLoading(), there's no equivalent call in the success path. Even thoughmessage.successmight implicitly handle this, it would be more explicit to ensure the loading state is properly cleared in all scenarios.deleteMenu(row.id) .then(() => { + hideLoading(); message.success({ content: $t('ui.actionMessage.deleteSuccess', [row.name]), key: 'action_process_msg', }); onRefresh(); })
138-138: Remove or utilize the empty div elementThis div has styling classes but contains no content. If it's meant to be a placeholder for future content, consider adding a comment. Otherwise, it can be removed to keep the template clean.
- <div class="items-center justify-end"></div>playground/src/api/system/menu.ts (1)
26-26: Consider removing the index signature withanytypeThe index signature
[key: string]: any;reduces type safety by allowing any property to be added with any type. This can lead to unexpected runtime errors if incorrect properties are accessed. Consider removing this line and explicitly defining all expected properties.export interface SystemMenu { - [key: string]: any; /** 后端权限标识 */ authCode: string; /** 子级 */ children?: SystemMenu[]; // ... other properties
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/backend-mock/api/system/menu/list.ts(1 hunks)apps/backend-mock/api/system/menu/name-exists.ts(1 hunks)apps/backend-mock/api/system/menu/path-exists.ts(1 hunks)apps/backend-mock/utils/mock-data.ts(1 hunks)packages/@core/ui-kit/form-ui/src/form-api.ts(1 hunks)packages/locales/src/langs/en-US/common.json(1 hunks)packages/locales/src/langs/en-US/ui.json(1 hunks)packages/locales/src/langs/zh-CN/common.json(1 hunks)packages/locales/src/langs/zh-CN/ui.json(1 hunks)playground/src/adapter/vxe-table.ts(3 hunks)playground/src/api/system/menu.ts(1 hunks)playground/src/locales/langs/en-US/system.json(1 hunks)playground/src/locales/langs/zh-CN/system.json(1 hunks)playground/src/router/routes/index.ts(1 hunks)playground/src/router/routes/modules/system.ts(1 hunks)playground/src/views/system/menu/data.ts(1 hunks)playground/src/views/system/menu/list.vue(1 hunks)playground/src/views/system/menu/modules/form.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Lint (windows-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (17)
packages/locales/src/langs/zh-CN/common.json (1)
19-21: Localization entries added correctlyThe new "yes" and "no" localization keys are properly added with correct Chinese translations, maintaining consistency with the existing entries in the file.
packages/locales/src/langs/en-US/common.json (1)
19-21: Localization entries added correctlyThe new "yes" and "no" localization keys are properly added with correct English translations, ensuring language consistency with the Chinese translations added in the zh-CN file.
playground/src/router/routes/modules/system.ts (1)
15-23: Menu management route added properlyThe new route for system menu management is correctly implemented with appropriate path, name, icon, and component import. The use of the translation function for the title supports internationalization.
packages/locales/src/langs/zh-CN/ui.json (1)
7-10: LGTM! Form validation messages added correctly.The new validation messages are properly localized in Chinese and follow the existing pattern with placeholders for dynamic content. These will be useful for the new menu management functionality.
packages/@core/ui-kit/form-ui/src/form-api.ts (1)
96-98: Enhanced type safety for getValues method.The addition of a generic type parameter to the
getValuesmethod is a good improvement that allows consumers to specify the expected return type, providing better type checking and IDE support.packages/locales/src/langs/en-US/ui.json (1)
7-10: LGTM! Form validation messages added correctly.The new validation messages are properly formatted in English and match the Chinese translations. They follow the existing pattern with appropriate placeholders for dynamic content.
playground/src/router/routes/index.ts (2)
38-45: Implementation of dynamic component keys for routingThe addition of
componentKeysis a good approach for dynamically accessing Vue components in the routing system. It provides a centralized way to reference all available view components without hardcoding paths.
47-47: Updated export statement to include componentKeysGood addition of the new
componentKeysconstant to the exports. This makes the component paths accessible to other parts of the application that need to reference these paths.apps/backend-mock/utils/mock-data.ts (1)
189-379: Well-structured mock menu dataThe
MOCK_MENU_LISTprovides a comprehensive structure for menu items with appropriate nesting and metadata. It includes different types of menu items (menu, catalog, button) and demonstrates various use cases like embedded views and external links.A few observations:
- Good use of hierarchical structure with parent-child relationships
- Proper inclusion of metadata like icons, titles, and ordering
- Comprehensive representation of different menu states (enabled/disabled)
- Inclusion of permission codes (authCode) for access control
This mock data will be very useful for the menu management demo page.
playground/src/adapter/vxe-table.ts (3)
8-8: Updated import to include the get utilityGood addition of the
getutility function import which will be used for safer property access in the CellTag renderer.
80-81: Enhanced property access safety in CellTag rendererThe changes improve the code in two important ways:
- Using
get(row, column.field)instead of direct property access ensures safe handling of nested properties- Using the nullish coalescing operator (
??) instead of logical OR is more precise, as it only falls back to the default when the value isnullorundefinedThese changes make the code more robust against potential runtime errors.
90-90: Improved object spread safety for undefined valuesUsing the nullish coalescing operator with an empty object (
tagItem ?? {}) ensures that even iftagItemis undefined, the spread operation will still work correctly without causing runtime errors.apps/backend-mock/api/system/menu/name-exists.ts (1)
1-16: Well-implemented utility for checking menu name existenceThe implementation correctly uses a map data structure to efficiently look up menu names. The recursive
getNamesfunction properly traverses the nested menu structure to populate the map.playground/src/locales/langs/en-US/system.json (1)
13-51: Well-structured internationalization additions for menu managementThe new menu section in the localization file is comprehensive and includes all necessary text entries for the menu management functionality. The keys follow a consistent naming pattern and provide thorough coverage of all UI elements mentioned in the PR objectives.
playground/src/views/system/menu/list.vue (1)
26-56: Grid configuration is well-structured for hierarchical menu displayThe grid configuration appropriately uses tree configuration with parent-child relationships, which is essential for menu hierarchy visualization. The proxy configuration correctly fetches data using the API and the toolbar is properly configured with custom buttons.
playground/src/api/system/menu.ts (2)
25-90: Well-structured menu interface with comprehensive metadata propertiesThe
SystemMenuinterface is thoroughly designed with all the necessary properties for a flexible menu system. The use of type constraints from the constant collections ensures type safety for enum-like values.
124-128: Good use of TypeScript's Omit utility for CRUD operationsThe
Omit<SystemMenuApi.SystemMenu, 'children' | 'id'>type ensures that the client doesn't send unnecessary or potentially problematic fields likechildrenoridduring creation. This is excellent type safety practice.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
playground/src/views/system/menu/modules/form.vue (6)
76-77: Simplify the empty input check.The condition could be simplified since an empty string will also have length 0.
- if (!input || input.length === 0) { + if (!input) { return true; }
465-466: Improve optional chaining in titleSuffix assignment.The code checks for
meta?.titlebut then assumes meta is defined when calling$t(formData.value.meta.title).- titleSuffix.value = formData.value.meta?.title - ? $t(formData.value.meta.title) - : ''; + titleSuffix.value = formData.value.meta?.title + ? $t(formData.value.meta?.title) + : '';
493-493: Consider using immutable operations instead ofdelete.Mutating the data object by deleting a property can be problematic in reactive environments. Consider using destructuring or spread operators.
- delete data.linkSrc; + const { linkSrc, ...dataWithoutLinkSrc } = data; + data = dataWithoutLinkSrc;
174-175: Simplify async/await usage in validation.The
asyncandawaitkeywords are redundant ifisMenuPathExistsalready returns a Promise.- .refine(async (value: string) => { - return await isMenuPathExists(value, formData.value?.id); + .refine((value: string) => { + return isMenuPathExists(value, formData.value?.id); }, $t('system.menu.activePathMustExist'))
142-143: Apply the same async/await simplification here.This is similar to the previous comment about redundant async/await usage.
- .refine( - async (value: string) => { - return !(await isMenuPathExists(value, formData.value?.id)); - }, + .refine( + (value: string) => { + return isMenuPathExists(value, formData.value?.id).then(exists => !exists); + },
59-60: Apply the same async/await simplification pattern here as well.This is another instance of redundant async/await usage.
- .refine( - async (value: string) => { - return !(await isMenuNameExists(value, formData.value?.id)); - }, + .refine( + (value: string) => { + return isMenuNameExists(value, formData.value?.id).then(exists => !exists); + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
playground/src/views/system/menu/data.ts(1 hunks)playground/src/views/system/menu/modules/form.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- playground/src/views/system/menu/data.ts
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Test (windows-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: Check (ubuntu-latest)
🔇 Additional comments (6)
playground/src/views/system/menu/modules/form.vue (6)
125-126: Fix the mismatch with'embedded'vs'embed'.This block references
data.type === 'embedded'to setdata.meta.iframeSrc, but in other parts of your code, you use'embed'. Align both references to avoid inconsistent or failing logic.Example fix:
- } else if (data.type === 'embedded') { + } else if (data.type === 'embed') { data.meta = { ...data.meta, iframeSrc: data.linkSrc }; }Also applies to: 157-159, 187-187, 200-201, 233-234, 248-249, 361-362, 457-458, 488-492
174-176: Verify the condition in activePath validation.The current implementation checks if the path already exists, but this seems counterintuitive for an input validation. Usually, you would check that a path doesn't exist for a new item.
Make sure this is the intended behavior:
.refine(async (value: string) => { return await isMenuPathExists(value, formData.value?.id); }, $t('system.menu.activePathMustExist'))
1-36: Great organization of imports and component setup.The imports are well-organized, and the component setup with TypeScript is clean and follows best practices.
37-432: Form schema is comprehensive and well-structured.The form schema includes all necessary fields for menu management with appropriate validation rules, conditional rendering, and internationalization support.
474-510: Form submission logic properly handles validation, API calls, and error cases.The form submission function includes proper validation, state management, and error handling with try/finally blocks to ensure loading states are reset even if errors occur.
517-521: Clean and responsive template structure.The template is clean and uses responsive layout based on screen size via the
isHorizontalcomputed property.
Description
添加菜单管理页面演示
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
New Features
Refactor