fix(block-refresh): fix block list not refresh when create update and delete #1056
Conversation
WalkthroughThe pull request introduces changes to the block plugin's category management system. The modifications focus on simplifying the logic for setting and updating categories in the block settings. A new reactive variable Changes
Sequence DiagramsequenceDiagram
participant UI as User Interface
participant Main as Main.vue
participant BlockSetting as blockSetting.jsx
UI->>Main: Change Category
Main->>BlockSetting: setCurrentCategory(val)
BlockSetting-->>BlockSetting: Update currentCategory
BlockSetting->>BlockSetting: updateBlockList()
BlockSetting-->>UI: Refresh Block List
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/plugins/block/src/js/blockSetting.jsx (2)
401-413: Add error handling for the API call.While the function correctly handles the category/group logic and sorting, it should include error handling for the API call to ensure robustness and provide feedback to users when the refresh fails.
export const updateBlockList = () => { let params = useBlock().shouldReplaceCategoryWithGroup() ? { groupId: currentCategory.value } : { categoryId: currentCategory.value } if (!currentCategory.value) { params = {} } const appId = getAppId() - fetchBlockList({ appId, ...params }).then((data) => { - const blockListDescByUpdateAt = data.sort((a, b) => new Date(b.updated_at) - new Date(a.updated_at)) - useBlock().setBlockList(blockListDescByUpdateAt) - }) + fetchBlockList({ appId, ...params }) + .then((data) => { + const blockListDescByUpdateAt = data.sort((a, b) => new Date(b.updated_at) - new Date(a.updated_at)) + useBlock().setBlockList(blockListDescByUpdateAt) + }) + .catch((error) => { + useNotify({ + type: 'error', + message: '刷新区块列表失败', + description: error.message + }) + }) }
535-537: Consider removing redundant refresh mechanism.The function uses both
updateBlockList()andisRefresh.value = truefor refresh. SinceupdateBlockList()already triggers a reactive update, theisRefreshflag might be redundant.- updateBlockList() - useBlock().isRefresh.value = true + updateBlockList()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/plugins/block/src/Main.vue(2 hunks)packages/plugins/block/src/js/blockSetting.jsx(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (3)
packages/plugins/block/src/js/blockSetting.jsx (2)
56-56: LGTM! Well-structured state management implementation.The reactive variable and its setter function are implemented following Vue's best practices.
Also applies to: 279-281
Line range hint
414-433: LGTM! Proper list refresh implementation after deletion.The function correctly refreshes the block list after successful deletion and includes proper error handling and user feedback.
packages/plugins/block/src/Main.vue (1)
351-352: LGTM! Clean and focused implementation.The changeCategory method is well-refactored to be more focused and maintainable, properly utilizing the new state management functions.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/plugins/materials/src/meta/block/src/BlockGroupPanel.vue (1)
169-169: Consolidate repeated refresh triggers.
isRefresh.valueis already set totrueearlier in the same flow. Using it twice in quick succession may be redundant and could potentially cause confusion. Consider removing or clarifying one of the triggers if it’s not needed.packages/plugins/block/src/BlockConfig.vue (1)
170-175: Handle empty groups or categories gracefully.If
block.groupsorblock.categoriesis empty, the fallback logic may result in an unexpected empty array or undefined identifier. Consider adding a default or a guard condition to prevent potential assignment errors.packages/plugins/block/src/js/blockSetting.jsx (3)
279-281: Check for invalid or empty category IDs.
setCurrentCategorysetscurrentCategory.valuewithout validation. IfcategoryIdis null or invalid, subsequent calls toupdateBlockListmay yield unexpected results. Consider adding a guard if needed.
537-538: Unify post-publish refresh logic.Similarly calling
updateBlockList()andisRefresh.value = trueensures the UI is in sync. Confirm all relevant create/update flows mirror this logic for uniform user experience.
724-724: Invoke “isRefresh” for uniformity.
updateBlockList()is called after new or updated blocks are saved. If other flows also triggerisRefresh.value = true, consider whether to apply the same pattern here to avoid partial states.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/common/component/BlockHistoryList.vue(1 hunks)packages/plugins/block/src/BlockConfig.vue(4 hunks)packages/plugins/block/src/js/blockSetting.jsx(9 hunks)packages/plugins/materials/src/meta/block/src/BlockGroupPanel.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (9)
packages/common/component/BlockHistoryList.vue (1)
2-2: Ensure that the “row-id” is indeed unique.Adding
row-id="id"helps the grid track rows by their unique identifier, which is important for row-level operations. However, please verify thatidvalues are truly unique across the data set to avoid potential rendering or state mismatch issues.packages/plugins/block/src/BlockConfig.vue (3)
41-41: Confirm that multiple selection is desired.By dynamically setting
:multiple="shouldReplaceCategoryWithGroup()", you switch between single and multiple category/group selection. Ensure that users will not be confused when the dropdown behavior changes based on this condition.
246-250: Use descriptive property keys for clarity.Using
'id'or'category_id'depending onshouldReplaceCategoryWithGroup()is correct. However, ensure that developers easily recognize which key is active and handle potential edge cases (e.g., an empty selection).
291-291: Propagating the condition is consistent.Exposing
shouldReplaceCategoryWithGroupin the returned setup object is logical for reusability across templates. The addition looks clean.packages/plugins/block/src/js/blockSetting.jsx (5)
13-13: Importing “ref” is necessary.Good catch integrating
reffrom Vue. This aligns with the newly introduced reactive variables below.
56-56: Initialize the reactive category to a known state.Defining
currentCategoryas an empty string by default is acceptable. Be mindful of how your watchers handle this initial empty state and whether any early calls might cause unintended requests.
401-413: Ensure robust error handling for “updateBlockList.”This function properly sets parameters based on
shouldReplaceCategoryWithGroup(). However, ifupdated_atis missing or invalid, the sort might fail. Consider verifying the field or adding a fallback before sorting.
427-428: Maintain consistency with other refresh mechanisms.Calling
updateBlockList()followed byisRefresh.value = trueis consistent with the approach in other methods like publishBlock. This ensures the UI remains in sync after block deletion.
659-659: Consider matching other operations’ patterns.While updating a block sets
isRefresh.value = true, other operations also callupdateBlockList(). Decide if an immediate block list refresh is needed here for consistency (or rely solely on watchers).
… delete;
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?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Refactor
Bug Fixes