refactor(Table): rename ProhibitEdit/Delete method name#5999
Conversation
Reviewer's GuideThis pull request refactors permission checking in the File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the Table component’s permission logic by renaming methods from CanEdit/CanDelete to ProhibitEdit/ProhibitDelete, inverting their semantics.
- Updated method calls in both the Table component and its unit tests
- Refactored internal helper functions to align with the new prohibition-based naming
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/UnitTest/Components/TableTest.cs | Updated unit tests to call ProhibitEdit/ProhibitDelete instead of CanEdit/CanDelete |
| src/BootstrapBlazor/Components/Table/Table.razor.Toolbar.cs | Refactored method names and their corresponding invocations for editing and deletion logic |
Comments suppressed due to low confidence (2)
test/UnitTest/Components/TableTest.cs:8687
- [nitpick] Consider renaming the helper method to 'IsEditProhibited' to make it clearer that it returns a boolean indicating whether editing is prohibited.
static bool ProhibitEdit(Table<Foo> @this)
src/BootstrapBlazor/Components/Table/Table.razor.Toolbar.cs:1006
- [nitpick] Consider renaming 'ProhibitEdit' to 'IsEditProhibited' (and similarly for 'ProhibitDelete') to indicate more clearly that these methods return a boolean status.
private bool ProhibitEdit() => (ShowExtendEditButtonCallback != null && !ShowExtendEditButtonCallback(SelectedRows[0])) || !ShowExtendEditButton || (DisableExtendEditButtonCallback != null && DisableExtendEditButtonCallback(SelectedRows[0])) || DisableExtendEditButton;
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5999 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 670 670
Lines 30624 30624
Branches 4356 4356
=========================================
Hits 30624 30624 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @ArgoZhang - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
|
|
||
| static bool CanEdit(Table<Foo> @this) | ||
| static bool ProhibitEdit(Table<Foo> @this) |
There was a problem hiding this comment.
suggestion (testing): Consider adding test cases for scenarios where editing/deleting is permitted.
Add tests confirming ProhibitEdit(cut.Instance) and ProhibitDelete(cut.Instance) return false when editing and deleting are allowed (e.g., buttons enabled and callbacks permit the action) to ensure the logic doesn’t wrongly block these operations.
Link issues
fixes #5998
Summary By Copilot
This pull request refactors the logic for determining edit and delete permissions in the
Tablecomponent by replacing theCanEditandCanDeletemethods withProhibitEditandProhibitDelete. Corresponding updates have been made to the unit tests to ensure consistency.Refactoring of permission logic in
Tablecomponent:CanEditwithProhibitEditandCanDeletewithProhibitDeletein theTable.razor.Toolbar.csfile. This change inverts the logic to focus on prohibitions rather than permissions. ([[1]](https://github.com/dotnetcore/BootstrapBlazor/pull/5999/files#diff-ca9e82d70b95de7204b56fbdace327d330eaef6777601f3d897b0e07c8c3eff8L548-R548),[[2]](https://github.com/dotnetcore/BootstrapBlazor/pull/5999/files#diff-ca9e82d70b95de7204b56fbdace327d330eaef6777601f3d897b0e07c8c3eff8L995-R995),[[3]](https://github.com/dotnetcore/BootstrapBlazor/pull/5999/files#diff-ca9e82d70b95de7204b56fbdace327d330eaef6777601f3d897b0e07c8c3eff8L1006-R1011))Updates to unit tests:
CanEditandCanDeletein theTableTest.csfile to useProhibitEditandProhibitDelete, ensuring the tests align with the new logic. ([[1]](https://github.com/dotnetcore/BootstrapBlazor/pull/5999/files#diff-3546a7a8520a1cc0576ca6a1077d468561e7b825b061c3d1aac2daae7a62a160L8654-R8655),[[2]](https://github.com/dotnetcore/BootstrapBlazor/pull/5999/files#diff-3546a7a8520a1cc0576ca6a1077d468561e7b825b061c3d1aac2daae7a62a160L8664-R8665),[[3]](https://github.com/dotnetcore/BootstrapBlazor/pull/5999/files#diff-3546a7a8520a1cc0576ca6a1077d468561e7b825b061c3d1aac2daae7a62a160L8683-R8690),[[4]](https://github.com/dotnetcore/BootstrapBlazor/pull/5999/files#diff-3546a7a8520a1cc0576ca6a1077d468561e7b825b061c3d1aac2daae7a62a160L8702-R8705))Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Refactor the permission logic in the Table component by renaming methods from
CanEditandCanDeletetoProhibitEditandProhibitDelete, inverting the permission check logic.Enhancements:
Tests: