feat(Table): add OnAfterDeleteAsync callback on DynamicContext mode#7832
feat(Table): add OnAfterDeleteAsync callback on DynamicContext mode#7832
Conversation
Reviewer's GuideRefactors delete callbacks in the Table component so that OnAfterDeleteAsync/OnAfterModifyAsync are centralized in a new helper and invoked consistently in DynamicContext and Excel delete flows, ensuring the after-delete callback also runs in DynamicContext mode. Sequence diagram for delete flow with centralized callbackssequenceDiagram
actor User
participant Table
participant DynamicContext
participant ExcelDeleteFlow as ExcelDelete
participant TriggerDeleteCallback
participant OnAfterDeleteAsync
participant OnAfterModifyAsync
User->>Table: DeleteAsync()
alt DynamicContext mode
Table->>DynamicContext: DeleteAsync(SelectedRows as IDynamicObject)
DynamicContext-->>Table: Task completed
Table->>Table: ResetDynamicContext()
Table->>TriggerDeleteCallback: TriggerDeleteCallback()
else Excel mode
Table->>ExcelDeleteFlow: InternalOnDeleteAsync()
ExcelDeleteFlow-->>Table: Task completed
Table->>TriggerDeleteCallback: TriggerDeleteCallback()
Table->>Table: QueryAsync()
else Normal mode
User->>Table: DeleteItemsAsync()
Table->>Table: confirm and delete items
Table->>TriggerDeleteCallback: TriggerDeleteCallback()
Table->>Table: SelectedRows.Clear()
Table->>Table: QueryAsync()
end
TriggerDeleteCallback->>OnAfterDeleteAsync: invoke with SelectedRows
OnAfterDeleteAsync-->>TriggerDeleteCallback: Task completed
TriggerDeleteCallback->>OnAfterModifyAsync: invoke()
OnAfterModifyAsync-->>TriggerDeleteCallback: Task completed
Class diagram for updated Table delete callbacksclassDiagram
class Table
Table : DynamicContext
Table : bool IsExcel
Table : List SelectedRows
Table : Func~IEnumerable~ OnAfterDeleteAsync
Table : Func OnAfterModifyAsync
Table : Task DeleteAsync()
Table : Task bool DeleteItemsAsync()
Table : Task TriggerDeleteCallback()
Table : void ResetDynamicContext()
class DynamicContext
DynamicContext : Task DeleteAsync(IEnumerable items)
class SelectedRows
class OnAfterDeleteAsync
class OnAfterModifyAsync
class TriggerDeleteCallback
class DeleteAsync
class DeleteItemsAsync
Table --> DynamicContext : uses
Table --> SelectedRows : maintains
Table --> OnAfterDeleteAsync : invokes
Table --> OnAfterModifyAsync : invokes
Table --> TriggerDeleteCallback : calls
DeleteAsync --> TriggerDeleteCallback : calls
DeleteItemsAsync --> TriggerDeleteCallback : calls
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider renaming
TriggerDeleteCallbackto something likeTriggerPostDeleteCallbacksorInvokeAfterDeleteAndModifyAsyncto better reflect that it triggers both delete and modify callbacks, not just delete. - Since
TriggerDeleteCallbackrelies onSelectedRows, it may be worth documenting that it must be called beforeSelectedRows.Clear()in all call sites to avoid future regressions if the order is changed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider renaming `TriggerDeleteCallback` to something like `TriggerPostDeleteCallbacks` or `InvokeAfterDeleteAndModifyAsync` to better reflect that it triggers both delete and modify callbacks, not just delete.
- Since `TriggerDeleteCallback` relies on `SelectedRows`, it may be worth documenting that it must be called before `SelectedRows.Clear()` in all call sites to avoid future regressions if the order is changed.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7832 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 764 764
Lines 34142 34147 +5
Branches 4701 4701
=========================================
+ Hits 34142 34147 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds support for invoking the existing post-delete callbacks (OnAfterDeleteAsync / OnAfterModifyAsync) when deleting rows in Table while using DynamicContext, aligning behavior across delete pathways.
Changes:
- Invoke after-delete callbacks for
DynamicContextandIsExceldelete flows. - Refactor repeated callback invocation into a shared
TriggerDeleteCallback()helper. - Bump
BootstrapBlazorpackage version to10.5.1-beta01.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/BootstrapBlazor/Components/Table/Table.razor.Toolbar.cs | Adds centralized after-delete callback triggering and wires it into DynamicContext/Excel deletion paths. |
| src/BootstrapBlazor/BootstrapBlazor.csproj | Updates package version to a beta prerelease. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ResetDynamicContext(); | ||
|
|
||
| // 触发删除回调方法 | ||
| await TriggerDeleteCallback(); | ||
|
|
There was a problem hiding this comment.
In the DynamicContext branch, ResetDynamicContext() runs before TriggerDeleteCallback(). ResetDynamicContext() ultimately calls ResetSelectedRows(QueryItems), which can replace SelectedRows with an empty list once the deleted items are no longer present. As a result, OnAfterDeleteAsync may receive the wrong (often empty) set of deleted rows. Capture SelectedRows.ToList() before resetting, and pass that snapshot into the callback (or invoke the callback before ResetDynamicContext()).
| ResetDynamicContext(); | |
| // 触发删除回调方法 | |
| await TriggerDeleteCallback(); | |
| // 触发删除回调方法 | |
| await TriggerDeleteCallback(); | |
| ResetDynamicContext(); |
| // 触发删除回调方法 | ||
| await TriggerDeleteCallback(); | ||
|
|
There was a problem hiding this comment.
This PR changes behavior by firing OnAfterDeleteAsync/OnAfterModifyAsync in DynamicContext mode, but there doesn't appear to be a unit test asserting these callbacks are invoked (and with the expected deleted rows) for DynamicContext deletion. Adding a test alongside the existing DynamicContext_* Table tests would prevent regressions here.
| <PropertyGroup> | ||
| <Version>10.5.0</Version> | ||
| <Version>10.5.1-beta01</Version> | ||
| </PropertyGroup> |
There was a problem hiding this comment.
The package version is changed to 10.5.1-beta01, but the PR description doesn’t mention a versioning/release intent. If this PR isn’t meant to publish a pre-release package, consider reverting this change; otherwise, document the reason for moving to a beta version in the PR description/release process notes.
Link issues
fixes #7831
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Enhancements: