EnxDev commented on code in PR #37773:
URL: https://github.com/apache/superset/pull/37773#discussion_r4002791986
##########
tests/integration_tests/core_tests.py:
##########
@@ -120,11 +121,18 @@ def test_slice_endpoint(self):
resp = self.client.get("/slice/-1/")
assert resp.status_code == 404
- def test_admin_only_menu_views(self):
- def assert_admin_view_menus_in(role_name, assert_func):
+ def test_admin_only_menu_views(self) -> None:
+ def assert_admin_view_menus_in(
+ role_name: str, assert_func: Callable[[str, list[str]], None]
+ ) -> None:
role = security_manager.find_role(role_name)
view_menus = [p.view_menu.name for p in role.permissions]
- assert_func("ResetPasswordView", view_menus)
+ if role_name == "Admin" and current_app.config.get(
+ "ENABLE_LEGACY_FAB_PASSWORD_VIEWS", False
+ ):
+ assert "ResetPasswordView" in view_menus
+ else:
+ assert "ResetPasswordView" not in view_menus
Review Comment:
Could we make this assertion independent of pre-existing role metadata? I
can reproduce a failure with:
`pytest tests/integration_tests/core_tests.py -k "admin_only_menu_views or
legacy_fab_password_views_are_not_registered" -q`
`ResetPasswordView` is still attached to Admin until
`sync_role_definitions()` runs, because skipping route registration does not
itself rewrite persisted role assignments. That makes this test depend on
whether the test database was created fresh or upgraded. The unit test below
already covers the role-sync behavior, so the cleanest option may be to keep
this integration test focused on the registered endpoints; alternatively,
explicitly run/setup role sync before asserting the role contents.
--
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]
