feat: Support merging categories and groups #907
Conversation
WalkthroughThe changes in this pull request introduce enhancements to the block management system within the application. A new property, Changes
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)
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 (
|
github-actions
bot
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (15)
packages/plugins/block/src/js/blockSetting.jsx (2)
607-615: Clarify variable naming for better readabilityIn the destructuring assignment,
categoriesis used even when it may represent groups. WhenshouldReplaceCategoryWithGroup()returnstrue,categoriesis assigned toextraParams.groups, which could be confusing. Consider renamingcategoriestogroupOrCategoryIdsor a similar name to reflect its dual purpose.
Line range hint
595-823: Refactor repeated conditional logic into helper functionsThe conditional logic using
useBlock().shouldReplaceCategoryWithGroup()to select between group and category functions and parameters is repeated multiple times throughout the code. Consider encapsulating this logic into helper functions to improve maintainability and reduce code duplication.For example:
function getFetchDataFunction() { return useBlock().shouldReplaceCategoryWithGroup() ? fetchGroups : fetchCategories; } function getExtraParams(categories) { return useBlock().shouldReplaceCategoryWithGroup() ? { groups: categories } : { categories }; } function getRequestFunction(isEdit) { const replaceCategoryWithGroup = useBlock().shouldReplaceCategoryWithGroup(); if (isEdit) { return replaceCategoryWithGroup ? updateGroup : updateCategory; } else { return replaceCategoryWithGroup ? createGroup : createCategory; } }Applying these helper functions can make the code cleaner and more maintainable.
packages/plugins/block/index.js (1)
23-25: Add JSDoc documentation for the new options object.The implementation looks good, but please consider adding documentation to explain the purpose and impact of the
mergeCategoriesAndGroupsoption.export default { ...metaData, apis: api, entry, metas: [BlockService], + /** + * Plugin configuration options + * @property {boolean} mergeCategoriesAndGroups - When true, replaces the category interface + * with the group interface in the block management system. Default: false + */ options: { mergeCategoriesAndGroups: false },packages/plugins/block/src/js/http.js (3)
78-81: Consider enhancing API documentation with JSDoc format.The comments clearly explain the purpose, but using JSDoc format would provide better documentation and IDE support.
-// 下面是区块分组的增删查改接口 -// 当 Block 插件的 options.mergeCategoriesAndGroups 为 true 时，将分类的接口全部替换成分组的接口 +/** + * Block group CRUD APIs + * These APIs replace the category APIs when Block plugin's options.mergeCategoriesAndGroups is true + */
87-88: Consider using PUT method for update operation.The current implementation uses POST with an /update/ path segment. Consider following REST conventions by using PUT method instead.
- getMetaApi(META_SERVICE.Http).post(`/material-center/api/block-groups/update/${id}`, params) + getMetaApi(META_SERVICE.Http).put(`/material-center/api/block-groups/${id}`, params)
91-92: Simplify create endpoint path.The /create segment in the path is redundant since POST method already implies creation.
- getMetaApi(META_SERVICE.Http).post('/material-center/api/block-groups/create', params) + getMetaApi(META_SERVICE.Http).post('/material-center/api/block-groups', params)packages/plugins/block/src/SaveNewBlock.vue (2)
27-29: Consider extracting the dynamic label to a computed property.While the current implementation works, having the conditional logic directly in the template makes it less maintainable. Consider moving this to a computed property for better readability and reusability.
+ const categoryLabel = computed(() => + shouldReplaceCategoryWithGroup() ? '请选择区块分组' : '请选择区块分类' + ) - <tiny-form-item :label="shouldReplaceCategoryWithGroup() ? '请选择区块分组' : '请选择区块分类'" prop="group"> + <tiny-form-item :label="categoryLabel" prop="group">
27-29: Consider updating form validation for consistency.While the label changes between "分组" and "分类", the form model and validation still use "group". Consider updating these for consistency with the new merged interface:
- The form data model uses
group- The validation rules don't include group/category validation
- The
changeCategorymethod name could be updatedConsider applying these changes:
const formData = reactive({ label: '', path: 'default', name_cn: '', - group: '' + blockGroup: '' // More descriptive name }) -const changeCategory = (value) => { +const changeBlockGroup = (value) => { const { id, category_id } = categoryList.value.find((item) => item.value === value) formData.path = category_id formData.categories = [id] } const rules = { name_cn: [{ required: true, message: '必填', trigger: 'blur' }], label: [ { pattern: REGEXP_BLOCK_NAME, message: '两个单词以上, 且是大写开头驼峰格式' }, { required: true, message: '必填', trigger: 'blur' } - ] + ], + blockGroup: [{ required: true, message: '必填', trigger: 'change' }] }Also applies to: 70-132
packages/plugins/block/src/CategoryEdit.vue (3)
27-28: Consider translating the Chinese comment to English.For better maintainability and consistency, consider translating the Chinese comment "分组不需要填写表单id字段" to English (e.g., "Category ID field is not required for groups").
- <!-- 分组不需要填写表单id字段 --> + <!-- Category ID field is not required for groups -->
63-75: Consider externalizing strings and improving maintainability.The current implementation could benefit from:
- Moving strings to an i18n configuration file
- Restructuring the logic to be more maintainable
Consider refactoring to:
+ import { useI18n } from 'vue-i18n' + + const { t } = useI18n() + + const baseLabels = { + group: { + text: t('block.group'), + nameInput: t('block.group.nameInput'), + nameInputPlaceholder: t('block.group.nameInputPlaceholder'), + validateErrMsg: t('block.group.validateErrMsg') + }, + category: { + text: t('block.category'), + nameInput: t('block.category.nameInput'), + nameInputPlaceholder: t('block.category.nameInputPlaceholder'), + validateErrMsg: t('block.category.validateErrMsg') + } + } + - const groupLabels = shouldReplaceCategoryWithGroup() - ? { - text: '分组', - nameInput: '分组名称', - nameInputPlaceholder: '请输入分组名称', - validateErrMsg: '分组名称不能重复！' - } - : { - text: '分类', - nameInput: '分类名称', - nameInputPlaceholder: '请输入分类名称', - validateErrMsg: '分类名称不能重复！' - } + const groupLabels = computed(() => + shouldReplaceCategoryWithGroup() ? baseLabels.group : baseLabels.category + )
108-111: Consider improvements to validation rules handling.Two suggestions for improvement:
- Translate the Chinese comment to English
- Make the rules more reactive using computed properties
Consider refactoring to:
- // 分组不需要填写表单id字段 - if (shouldReplaceCategoryWithGroup()) { - rules.categoryId = [] - } + // Category ID validation is not required for groups + const rules = computed(() => ({ + name: [ + { required: true, message: '必填', trigger: 'blur' }, + { pattern: REGEXP_GROUP_NAME, message: '只能包含大小写字母、数字和特殊字符', trigger: 'change' }, + { + validator: validateGroup, + trigger: 'blur' + } + ], + categoryId: shouldReplaceCategoryWithGroup() ? [] : [ + { required: true, message: '必填', trigger: 'blur' } + ] + }))packages/plugins/block/src/BlockConfig.vue (1)
125-125: Consider adding JSDoc documentation.The
shouldReplaceCategoryWithGroupimport is correctly implemented. However, consider adding JSDoc documentation to explain the purpose and return type of this method.+/** + * Destructure block management utilities + * @typedef {Object} BlockUtils + * @property {Function} getCategoryList - Retrieves the list of categories + * @property {Function} shouldReplaceCategoryWithGroup - Determines if category labels should be replaced with group labels + */ const { getCategoryList, shouldReplaceCategoryWithGroup } = useBlock()packages/plugins/block/src/Main.vue (2)
330-341: Consider moving labels to i18n configurationWhile the implementation is correct, consider moving these labels to a separate i18n configuration file for better maintainability and easier localization.
Example structure:
+ // i18n/block.js + export default { + en: { + group: { + selectPlaceholder: 'Show all groups by default', + deletePrompt: 'Are you sure you want to delete this block group?', + deleteTitle: 'Delete Group' + }, + category: { + selectPlaceholder: 'Show all categories by default', + deletePrompt: 'Are you sure you want to delete this block category?', + deleteTitle: 'Delete Category' + } + } + }
Line range hint
343-349: Consider adding type validation for the value parameterWhile the implementation correctly handles the parameter naming, consider adding type validation for the
valparameter to prevent potential runtime issues.const changeCategory = (val) => { + if (val && typeof val !== 'string' && typeof val !== 'number') { + console.warn('Invalid value type for category/group ID'); + return; + } let params = useBlock().shouldReplaceCategoryWithGroup() ? { groupId: val } : { categoryId: val } if (!val) { params = {} } updateBlockList(params) }packages/plugins/block/src/composable/useBlock.js (1)
720-723: Add error handling for configuration accessThe function logic is correct but could benefit from defensive programming to handle edge cases.
Consider adding error handling:
const shouldReplaceCategoryWithGroup = () => { - const { mergeCategoriesAndGroups } = getOptions(meta.id) - return mergeCategoriesAndGroups + try { + const { mergeCategoriesAndGroups = false } = getOptions(meta.id) || {} + return mergeCategoriesAndGroups + } catch (error) { + console.warn('Failed to get mergeCategoriesAndGroups option:', error) + return false + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
packages/plugins/block/index.js(1 hunks)packages/plugins/block/src/BlockConfig.vue(4 hunks)packages/plugins/block/src/CategoryEdit.vue(6 hunks)packages/plugins/block/src/Main.vue(5 hunks)packages/plugins/block/src/SaveNewBlock.vue(3 hunks)packages/plugins/block/src/composable/useBlock.js(3 hunks)packages/plugins/block/src/js/blockSetting.jsx(7 hunks)packages/plugins/block/src/js/http.js(1 hunks)
🔇 Additional comments (22)
packages/plugins/block/src/js/blockSetting.jsx (7)
45-49: Importing group management functions
The group management functions (fetchGroups, createGroup, updateGroup, deleteGroup) are correctly imported, aligning with the new feature to merge categories and groups.
595-599: Conditional fetching of categories or groups
The fetchData function is appropriately assigned based on useBlock().shouldReplaceCategoryWithGroup(), ensuring that either groups or categories are fetched as needed.
663-669: Consistent handling of extra parameters
The conditional assignment to extraParams based on shouldReplaceCategoryWithGroup() is consistent with the changes made elsewhere and correctly updates either groups or categories.
680-681: Including label in update parameters
By adding label to the parameters spread, the block's label is ensured to be updated during the block update process.
804-805: Conditional selection of request function
The selection between updateGroup and updateCategory based on replaceCategoryWithGroup correctly aligns the update operation with the current mode (group or category).
808-813: Adjusting parameters for create operation
When creating a new category or group (!isEdit), the code correctly assigns params.category_id if not replacing categories with groups and selects the appropriate request function. This ensures that new entities are created appropriately based on the configuration.
822-823: Simplifying delete function selection
The deletion function deleteFn is correctly determined based on useBlock().shouldReplaceCategoryWithGroup(), ensuring that the appropriate delete operation is performed.
packages/plugins/block/index.js (1)
23-25: Verify the option's usage across components.
Let's ensure this option is properly consumed by the affected components.
✅ Verification successful
Let me check the actual implementation and usage of this option to ensure it's properly handled.
Let me check how this option affects the UI components and API calls to ensure complete verification.
Option is properly integrated across components
The verification shows that the mergeCategoriesAndGroups option is correctly implemented and consumed:
- In
useBlock.js, it's properly retrieved from plugin options and exposed viashouldReplaceCategoryWithGroup() - UI components (CategoryEdit.vue, SaveNewBlock.vue, BlockConfig.vue) correctly adapt their behavior:
- Form fields and labels switch between "分类" (category) and "分组" (group)
- CategoryEdit conditionally shows/hides the categoryId field
- SaveNewBlock and BlockConfig use appropriate labels based on the option
- HTTP layer (http.js) provides both category and group API endpoints, with proper comments indicating the option's effect
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check how the mergeCategoriesAndGroups option is used across components
# Search for direct references to the option
echo "Direct option references:"
rg -l "mergeCategoriesAndGroups"
# Search for the shouldReplaceCategoryWithGroup function mentioned in the summary
echo -e "\nUsage of shouldReplaceCategoryWithGroup:"
rg -l "shouldReplaceCategoryWithGroup"
# Look for UI text changes between category/group
echo -e "\nDynamic category/group labels:"
rg -l "分[类组]"
Length of output: 2399
Script:
#!/bin/bash
# Check the implementation details in key files
echo "Content of useBlock.js where option is used:"
rg -A 5 "mergeCategoriesAndGroups" "packages/plugins/block/src/composable/useBlock.js"
echo -e "\nContent of http.js where option is used:"
rg -A 5 "mergeCategoriesAndGroups" "packages/plugins/block/src/js/http.js"
echo -e "\nImplementation of shouldReplaceCategoryWithGroup:"
ast-grep --pattern 'const shouldReplaceCategoryWithGroup = $_'
Length of output: 1287
Script:
#!/bin/bash
# Check the usage in UI components and API integration
echo "Usage in CategoryEdit.vue:"
rg -B 2 -A 5 "shouldReplaceCategoryWithGroup" "packages/plugins/block/src/CategoryEdit.vue"
echo -e "\nUsage in SaveNewBlock.vue:"
rg -B 2 -A 5 "shouldReplaceCategoryWithGroup" "packages/plugins/block/src/SaveNewBlock.vue"
echo -e "\nUsage in BlockConfig.vue:"
rg -B 2 -A 5 "shouldReplaceCategoryWithGroup" "packages/plugins/block/src/BlockConfig.vue"
echo -e "\nAPI calls related to categories/groups:"
rg -A 3 "fetchGroups|fetchCategories" "packages/plugins/block/src/js/http.js"
Length of output: 3294
packages/plugins/block/src/js/http.js (2)
83-84: LGTM: Fetch groups implementation
The implementation correctly includes the 'from: block' parameter to differentiate block groups from other group types.
82-96: Verify API compatibility between category and group endpoints.
Since these group APIs are meant to replace category APIs when mergeCategoriesAndGroups is true, we should verify that the request/response formats are compatible.
✅ Verification successful
Based on the shell script results, I can now generate the final response as there's sufficient information to verify the API compatibility:
API compatibility between category and group endpoints is maintained
The codebase shows proper handling of both category and group APIs with compatible request/response formats:
- Both APIs follow similar URL patterns (
/material-center/api/block-categoriesvs/material-center/api/block-groups) - The switching logic in
blockSetting.jsxdemonstrates parameter compatibility:fetchGroups/fetchCategoriesboth acceptparamswithappIdupdateGroup/updateCategoryboth handleidand remaining params similarlycreateGroup/createCategoryboth accept the same params structuredeleteGroup/deleteCategoryboth accept an ID parameter
The conditional switching is properly implemented through shouldReplaceCategoryWithGroup() which checks the mergeCategoriesAndGroups option.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Compare the API endpoints and their usage in the codebase
# Check for category API usage patterns
echo "Category API usage patterns:"
rg -A 2 "fetchCategories|updateCategory|createCategory|deleteCategory"
# Check for group API usage patterns
echo "Group API usage patterns:"
rg -A 2 "fetchGroups|updateGroup|createGroup|deleteGroup"
# Check for any conditional logic between categories and groups
echo "Conditional logic between categories and groups:"
rg -A 5 "mergeCategoriesAndGroups"
Length of output: 14470
packages/plugins/block/src/SaveNewBlock.vue (1)
70-70: Verify the integration with useBlock hook.
The changes look good for supporting the new mergeCategoriesAndGroups feature. However, let's verify that the useBlock hook properly implements this functionality.
Also applies to: 130-131
✅ Verification successful
Based on the verification results, I can now provide a final response:
The shouldReplaceCategoryWithGroup implementation is correctly integrated
The implementation properly reads the mergeCategoriesAndGroups option from the block's metadata and is consistently used across multiple components. The function is:
- Correctly implemented in useBlock.js
- Used appropriately in SaveNewBlock.vue and other components
- Properly controls the UI between category and group modes based on the configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of shouldReplaceCategoryWithGroup in useBlock
# Check the useBlock implementation
ast-grep --pattern 'export const useBlock = () => {
$$$
const shouldReplaceCategoryWithGroup = $$$
$$$
}'
# Check for any other components using this method
rg "shouldReplaceCategoryWithGroup" --type vue
Length of output: 188
Script:
#!/bin/bash
# Let's try a different approach to find the useBlock implementation and its usage
# Search for useBlock implementation
rg -l "export.*useBlock"
# Search for shouldReplaceCategoryWithGroup usage with proper file extensions
rg "shouldReplaceCategoryWithGroup" -g "*.{js,jsx,ts,tsx,vue}"
# Find all Vue files to ensure we're searching correctly
fd -e vue
Length of output: 18958
Script:
#!/bin/bash
# Let's examine the useBlock implementation and shouldReplaceCategoryWithGroup function
# Check the implementation in useBlock.js
rg -A 10 "shouldReplaceCategoryWithGroup = \(\)" packages/plugins/block/src/composable/useBlock.js
# Check the full useBlock export to see how it's structured
rg -A 20 "export const useBlock" packages/plugins/block/src/composable/useBlock.js
Length of output: 453
packages/plugins/block/src/CategoryEdit.vue (3)
20-25: LGTM! Dynamic form labels implementation looks good.
The form item successfully implements dynamic labeling based on whether it's a category or group, maintaining backward compatibility while supporting the new merged interface.
54-54: LGTM! Proper usage of the useBlock hook.
The addition of shouldReplaceCategoryWithGroup to the destructured imports is clean and follows composition API best practices.
Line range hint 1-170: Verify consistency across all category/group related files.
Let's ensure that all related files are consistently implementing the category/group merge functionality.
✅ Verification successful
Category/Group merge implementation is consistent across the codebase
Based on the verification results, the implementation of merging categories and groups is consistent across all relevant files. The key findings show:
-
The
shouldReplaceCategoryWithGroup()function is used consistently throughout the codebase to control the category/group merge behavior:- In UI components (
CategoryEdit.vue,BlockConfig.vue,Main.vue,SaveNewBlock.vue) - In business logic (
blockSetting.jsx,useBlock.js) - For API calls and data handling
- In UI components (
-
The implementation properly handles:
- Conditional rendering of UI elements
- API endpoint selection
- Data structure transformations
- Labels and text content
- Form validation rules
-
The changes maintain backward compatibility by:
- Using feature flags
- Preserving both category and group-related code paths
- Consistently applying conditional logic
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistency in category/group implementation across files
# Find all files that might contain category/group related code
echo "Checking for category/group related files..."
rg -l "分[类组]|category|group" --type vue --type js
# Check for any remaining hardcoded Chinese strings related to categories/groups
echo "Checking for hardcoded Chinese strings..."
rg "分[类组]" --type vue --type js
# Check for consistent usage of shouldReplaceCategoryWithGroup
echo "Checking shouldReplaceCategoryWithGroup usage..."
rg "shouldReplaceCategoryWithGroup" --type vue --type js -A 2
Length of output: 530
Script:
#!/bin/bash
# Let's try without specifying file types and use more generic search
echo "Checking for category/group related files..."
rg -l "分[类组]|category|group"
echo -e "\nChecking for hardcoded Chinese strings..."
rg "分[类组]"
echo -e "\nChecking shouldReplaceCategoryWithGroup usage..."
rg "shouldReplaceCategoryWithGroup" -A 2
Length of output: 24329
packages/plugins/block/src/BlockConfig.vue (2)
27-27: LGTM! Dynamic labeling implementation looks good.
The dynamic binding for labels and placeholders correctly implements the category/group merging feature while maintaining backward compatibility.
Also applies to: 32-32
125-125: Verify integration with block management page.
The changes look good, but let's verify the integration with the block management page to ensure consistent labeling.
Also applies to: 250-259
✅ Verification successful
Integration with block management page is properly handled
The verification shows consistent implementation across the codebase:
- The
shouldReplaceCategoryWithGroupfunction is used to conditionally render UI labels - All UI text properly switches between "区块分组" (Block Group) and "区块分类" (Block Category) based on the flag
- Default group/category labels are consistently handled
- API endpoints for both categories and groups are properly maintained with clear documentation about the merge behavior
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent label usage across block management files
# Search for hardcoded category/group labels that might need to be updated
rg -g '*.{vue,js,ts}' -A 2 -B 2 '区块分[类组]|默认分[类组]'
# Search for other components using shouldReplaceCategoryWithGroup
ast-grep --pattern 'shouldReplaceCategoryWithGroup'
Length of output: 13932
packages/plugins/block/src/Main.vue (3)
19-19: LGTM: Dynamic labeling implementation looks good!
The template changes correctly implement dynamic labeling using the groupLabels object, which allows seamless switching between category and group terminology based on configuration.
Also applies to: 51-51
438-439: LGTM: Return statement properly updated
The new properties are correctly added to the component's return object.
Line range hint 1-554: Verify consistent terminology usage across the codebase
Let's ensure all category/group related terminology is consistently handled.
✅ Verification successful
Based on the search results, I can now provide a final response:
Terminology usage is properly handled through dynamic configuration
The code correctly handles the category/group terminology through the shouldReplaceCategoryWithGroup() method, which dynamically determines whether to use "category" or "group" terminology. This is consistently applied across:
- Label text via
groupLabelsobject - API parameters via conditional
groupId/categoryId - Component naming and functionality
The implementation allows for flexible terminology switching without hardcoded terms, making it maintainable and configurable.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any hardcoded category/group terminology that might need to be made dynamic
# Search for hardcoded category terminology
echo "Checking for hardcoded category terminology..."
rg -i "category|分类" --type vue --type js
# Search for hardcoded group terminology
echo "Checking for hardcoded group terminology..."
rg -i "group|分组" --type vue --type js
# Search for related component usage
echo "Checking for related component usage..."
ast-grep --pattern 'CategoryEdit'
Length of output: 461
Script:
#!/bin/bash
# Search for hardcoded category/group terminology using ripgrep with proper file extensions
echo "Checking for hardcoded category terminology..."
rg -i "category|分类" -t js -t jsx -t vue
echo "Checking for hardcoded group terminology..."
rg -i "group|分组" -t js -t jsx -t vue
# Search for CategoryEdit component usage and implementation
echo "Checking CategoryEdit component implementation..."
rg -l "CategoryEdit" -t js -t jsx -t vue
Length of output: 514
Script:
#!/bin/bash
# Search using just the basic file extensions and simpler patterns
echo "Searching for category/classification related terms..."
rg -i "category|分类|group|分组" .
echo "Searching for CategoryEdit component..."
rg -l "CategoryEdit" .
# Look for specific Vue component imports and usage
echo "Searching for Vue component declarations..."
rg "components:\s*{[^}]*CategoryEdit[^}]*}" .
Length of output: 157756
Script:
#!/bin/bash
# Search for category/group terminology and CategoryEdit component with simpler patterns
echo "Searching for category/group terminology..."
rg -i "category|group" packages/plugins/block/src/Main.vue
echo "Searching for CategoryEdit component..."
rg "CategoryEdit" packages/plugins/block/src/Main.vue
# Look for shouldReplaceCategoryWithGroup usage
echo "Searching for shouldReplaceCategoryWithGroup usage..."
rg "shouldReplaceCategoryWithGroup" packages/plugins/block/src/Main.vue
Length of output: 3451
packages/plugins/block/src/composable/useBlock.js (3)
30-30: LGTM: Import addition is well-placed
The addition of getOptions import is properly grouped with related imports from the meta-register module.
768-769: LGTM: Exports are well-organized
The new functions are properly exported and maintain alphabetical ordering in the exports list.
33-33: Verify meta configuration file
The import path suggests accessing the plugin's meta configuration. Let's verify the file exists and contains the required configuration.
✅ Verification successful
Let me gather more information about the meta file structure.
Meta configuration file is valid and properly structured
The meta file exists at the correct location and contains the required plugin configuration with a valid ID that's used by the useBlock composable to fetch options. The mergeCategoriesAndGroups option is properly referenced in the codebase and its default value is set to false in the plugin's index file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify meta file exists and contains mergeCategoriesAndGroups option
# Check if meta file exists
fd --type f "meta.(js|ts)" --parent-dir "packages/plugins/block"
# Check content of meta file for mergeCategoriesAndGroups configuration
rg -A 5 "mergeCategoriesAndGroups" "packages/plugins/block"
Length of output: 1643
Script:
#!/bin/bash
# Check for meta file in the block plugin directory
fd "meta" packages/plugins/block
# Show content of potential meta files
rg -l "export" packages/plugins/block/meta.js || true
cat packages/plugins/block/meta.js 2>/dev/null || true
Length of output: 313
…to mergeCategoriesAndGroups
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
packages/plugins/block/src/Main.vue (2)
331-342: Consider moving string literals to i18n configurationWhile the implementation is correct, consider moving these UI strings to a dedicated i18n configuration file for better maintainability and easier localization.
Example structure:
+ // i18n/zh-CN.js + export const blockManagement = { + category: { + selectPlaceholder: '默认展示全部分类', + deletePrompt: '您确定删除该区块分类吗？', + deleteTitle: '删除分类' + }, + group: { + selectPlaceholder: '默认展示全部分组', + deletePrompt: '您确定删除该区块分组吗？', + deleteTitle: '删除分组' + } + }
Line range hint
344-351: Consider adding JSDoc for better clarityThe function handles both category and group changes, but its behavior could be more explicit through documentation.
Consider adding JSDoc:
+ /** + * Handles the change of category/group selection + * @param {string|null} val - The selected category/group ID or null when cleared + */ const changeCategory = (val) => { let params = useBlock().shouldReplaceCategoryWithGroup() ? { groupId: val } : { categoryId: val } if (!val) { params = {} } updateBlockList(params) }packages/plugins/block/src/js/blockSetting.jsx (3)
607-615: Simplify the parameter construction logic.The current implementation can be made more concise while maintaining readability.
- const { categories, ...rest } = block - const extraParams = {} - - if (useBlock().shouldReplaceCategoryWithGroup()) { - extraParams.groups = categories - } else { - extraParams.categories = categories - } - - const params = { ...rest, ...extraParams, created_app } + const { categories, ...rest } = block + const params = { + ...rest, + created_app, + [useBlock().shouldReplaceCategoryWithGroup() ? 'groups' : 'categories']: categories + }
663-669: Apply the same simplification to update parameters.For consistency, apply the same concise parameter construction pattern here.
- const extraParams = {} - if (useBlock().shouldReplaceCategoryWithGroup()) { - extraParams.groups = categories - } else { - extraParams.categories = categories - } + const categoryParam = { + [useBlock().shouldReplaceCategoryWithGroup() ? 'groups' : 'categories']: categories + }
810-817: Clarify the category_id parameter logic.The condition for adding
category_idcould be more explicit about its purpose.- if (!replaceCategoryWithGroup) { - params.category_id = categoryId - } + // Only add category_id for category interface, not needed for group interface + if (!replaceCategoryWithGroup && categoryId) { + params.category_id = categoryId + }packages/plugins/block/src/SaveNewBlock.vue (3)
27-30: Consider aligning method and property names with merged terminology.While the label is now dynamic, the underlying implementation still uses category-specific terminology (e.g.,
changeCategory,categoryList). This could lead to confusion in the codebase.Consider renaming for consistency:
- <tiny-form-item :label="shouldReplaceCategoryWithGroup() ? '区块分组' : '区块分类'" prop="group"> - <tiny-select v-model="formData.group" :options="categoryList" placeholder="请选择" @change="changeCategory"> + <tiny-form-item :label="shouldReplaceCategoryWithGroup() ? '区块分组' : '区块分类'" prop="blockGroup"> + <tiny-select v-model="formData.blockGroup" :options="groupList" placeholder="请选择" @change="handleGroupChange">
130-132: LGTM! Consider adding JSDoc for the new method.The addition of
shouldReplaceCategoryWithGroupto the return object is correct. Consider adding JSDoc documentation to describe its purpose and return type.return { formData, categoryList, formRef, rules, addBlock, cancel, changeCategory, visible, + /** @returns {boolean} Whether to replace category terminology with group terminology */ shouldReplaceCategoryWithGroup }
Line range hint
1-143: General improvements needed for the component.
- Tests are missing as noted in PR objectives
- Error messages should use i18n for internationalization
- There's a questionable comment about materials activation that should be resolved
Would you like me to:
- Generate test cases for this component?
- Provide an example of i18n implementation for the messages?
- Help investigate the materials activation concern?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/plugins/block/src/CategoryEdit.vue(6 hunks)packages/plugins/block/src/Main.vue(5 hunks)packages/plugins/block/src/SaveNewBlock.vue(3 hunks)packages/plugins/block/src/composable/useBlock.js(3 hunks)packages/plugins/block/src/js/blockSetting.jsx(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/plugins/block/src/composable/useBlock.js
- packages/plugins/block/src/CategoryEdit.vue
🔇 Additional comments (6)
packages/plugins/block/src/Main.vue (3)
19-19: LGTM: Dynamic placeholder text implementation
The placeholder text now correctly adapts based on whether categories are being replaced with groups.
365-365: LGTM: Proper implementation of dynamic delete confirmation
The delete confirmation dialog correctly uses dynamic labels based on the current context (category/group).
439-440: LGTM: Proper exposure of required properties
The necessary properties are correctly exposed to the template.
packages/plugins/block/src/js/blockSetting.jsx (2)
45-49: LGTM! Import statements are well-organized.
The new group-related function imports maintain consistency with the existing import pattern.
828-829: LGTM! Clean implementation with proper feedback.
The dynamic function selection is well-implemented and includes appropriate success notification.
packages/plugins/block/src/SaveNewBlock.vue (1)
70-70: Verify category-specific methods handle merged functionality.
The imported methods (createEmptyBlock, createBlock, getCategoryList) still use category-specific naming. Ensure these methods handle both category and group scenarios correctly when mergeCategoriesAndGroups is enabled.
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
区块管理的区块分类接口与物料的区块分组接口响应基本一致，支持将分类接口和分组接口，合并成区块分组接口，实际上是将分类接口替换成分组接口
What is the current behavior?
What is the new behavior?
区块插件增加了
options.mergeCategoriesAndGroups配置项，来决定是否合并分类和分组接口。mergeCategoriesAndGroups为false（默认值），保持原来的逻辑不变，即分类和分组接口都会调用mergeCategoriesAndGroups为true，将分类接口替换成分组接口。并且将区块管理页面的分类文本全部替换成了分组文本注册表示例：
export default { // ... plugins: [ Materials, Tree, Page, - Block, + [Block, { options: { ...Block.options, mergeCategoriesAndGroups: true } }], Datasource, Bridge, I18n, Script, State, Schema, Help, Robot ], // ... }Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation