Open
Conversation
Comprehensive analysis of the permission system introduced in 1.0a20 through 1.0a24, covering architecture, security, performance, and design concerns across 12 identified issues with prioritized recommendations. https://claude.ai/code/session_013EkyroQKPhcjdMbpHc9g4X
Detailed design for extracting build_cascading_ctes(), collect_permission_rules(), and build_restriction_filter() to replace three separate implementations with one shared SQL builder. Includes migration plan and handles the include_is_private complication. https://claude.ai/code/session_013EkyroQKPhcjdMbpHc9g4X
Three changes:
1. Rewrite test_utils_permissions.py to exercise production code paths
(allowed_resources / allowed) instead of the test-only
resolve_permissions_from_catalog function. Tests now register plugins
via ds.pm.register and call the real Datasette methods.
2. Remove resolve_permissions_from_catalog, resolve_permissions_with_candidates,
and build_rules_union from datasette/utils/permissions.py. These were
only used by tests and implemented the cascading logic a third time,
independently of the two production implementations.
3. Fix bug in gather_permission_sql_from_hooks where empty params dicts
({}) would cause framework-injected params (:actor_id, :actor, :action)
to be silently lost. The expression `params = permission_sql.params or {}`
creates a new dict when params is {} (falsy), so setdefault writes to a
throwaway dict. Fixed by explicitly checking `is None`.
https://claude.ai/code/session_013EkyroQKPhcjdMbpHc9g4X
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2634 +/- ##
==========================================
+ Coverage 90.37% 90.54% +0.16%
==========================================
Files 52 52
Lines 7983 7936 -47
==========================================
- Hits 7215 7186 -29
+ Misses 768 750 -18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Comprehensive analysis of the permission system introduced in 1.0a20
through 1.0a24, covering architecture, security, performance, and
design concerns across 12 identified issues with prioritized
recommendations.
https://claude.ai/code/session_013EkyroQKPhcjdMbpHc9g4X
📚 Documentation preview 📚: https://datasette--2634.org.readthedocs.build/en/2634/