rusackas commented on code in PR #40527:
URL: https://github.com/apache/superset/pull/40527#discussion_r3432187382
##########
superset/views/groups.py:
##########
@@ -25,7 +25,7 @@
class GroupsListView(BaseSupersetView):
route_base = "/"
- class_permission_name = "security"
+ class_permission_name = "Security"
Review Comment:
The catch is that "security" lowercase in these five views is the actual
outlier. Every other view uses its properly cased name, and the
ADMIN_ONLY_VIEW_MENUS / PUBLIC_EXCLUDED_VIEW_MENUS both key off "Security". So
this isn't inventing a value to please the UI so much as bringing five
stragglers in line with the name everything else already expects.
lower() would be tempting, but the case-sensitive comparisons aren't in one
place. Some are in our `_is_admin_only`, but plenty live in FAB itself (the
menu builder, add_permission_view_menu...) which we'd have to override as well.
Also, normalizing it in the code while the DB stores it in the other casing has
the same collation quirk that caused the bug, so it feels more like a ban-aid.
Existing installs already have the stray row too, so the migration's needed
either way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
