bito-code-review[bot] commented on code in PR #37773:
URL: https://github.com/apache/superset/pull/37773#discussion_r4078284644
##########
tests/unit_tests/security/manager_test.py:
##########
@@ -4320,6 +4320,68 @@ def
test_request_loader_rejects_invalid_guest_token_before_bearer(
verify_jwt.assert_not_called()
[email protected](
+
"enable_legacy_password_views,enable_force_password_change,expected_registered",
+ [
+ (False, False, {"NonPasswordView"}),
+ (False, True, {"NonPasswordView", "ResetMyPasswordView"}),
+ (
+ True,
+ False,
+ {"NonPasswordView", "ResetMyPasswordView", "ResetPasswordView"},
+ ),
+ ],
+)
+def test_skip_legacy_fab_password_view_registration_keeps_forced_change_target(
+ app_context: None,
+ enable_legacy_password_views: bool,
+ enable_force_password_change: bool,
+ expected_registered: set[str],
+) -> None:
+ """Forced password changes require the self-service reset view."""
+ from flask import current_app
+ from flask_appbuilder.security.views import ResetMyPasswordView,
ResetPasswordView
Review Comment:
<!-- Bito Reply -->
The changes correctly address the review suggestion by moving the imports to
the module level and removing the duplicate import. This aligns with the
repository's import rules and maintains clean code structure.
##########
tests/unit_tests/security/manager_test.py:
##########
@@ -4320,6 +4320,68 @@ def
test_request_loader_rejects_invalid_guest_token_before_bearer(
verify_jwt.assert_not_called()
[email protected](
+
"enable_legacy_password_views,enable_force_password_change,expected_registered",
+ [
+ (False, False, {"NonPasswordView"}),
+ (False, True, {"NonPasswordView", "ResetMyPasswordView"}),
+ (
+ True,
+ False,
+ {"NonPasswordView", "ResetMyPasswordView", "ResetPasswordView"},
+ ),
+ ],
+)
+def test_skip_legacy_fab_password_view_registration_keeps_forced_change_target(
+ app_context: None,
+ enable_legacy_password_views: bool,
+ enable_force_password_change: bool,
+ expected_registered: set[str],
+) -> None:
+ """Forced password changes require the self-service reset view."""
+ from flask import current_app
+ from flask_appbuilder.security.views import ResetMyPasswordView,
ResetPasswordView
+
+ class NonPasswordView:
+ pass
+
+ registered: list[str] = []
+
+ def add_view_no_menu(baseview: type[Any], *args: Any, **kwargs: Any) ->
type[Any]:
+ registered.append(baseview.__name__)
+ return baseview
+
+ fake_appbuilder = SimpleNamespace(add_view_no_menu=add_view_no_menu)
+ sm = SupersetSecurityManager.__new__(SupersetSecurityManager)
+ sm.appbuilder = fake_appbuilder
+
+ previous_config = {
+ "ENABLE_LEGACY_FAB_PASSWORD_VIEWS": current_app.config[
+ "ENABLE_LEGACY_FAB_PASSWORD_VIEWS"
+ ],
+ "ENABLE_FORCE_PASSWORD_CHANGE": current_app.config[
+ "ENABLE_FORCE_PASSWORD_CHANGE"
+ ],
+ }
+ current_app.config["ENABLE_LEGACY_FAB_PASSWORD_VIEWS"] = (
+ enable_legacy_password_views
+ )
+ current_app.config["ENABLE_FORCE_PASSWORD_CHANGE"] =
enable_force_password_change
+
+ original_add_view_no_menu: Callable[..., Any] =
fake_appbuilder.add_view_no_menu
+ try:
+ original_add_view_no_menu =
sm._skip_legacy_fab_password_view_registration()
Review Comment:
<!-- Bito Reply -->
Understood. Removing the redundant initializer and the unused import is the
correct approach to clean up the code in that test case.
--
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]
