Made acting_user a mandatory keyword argument where present. #15958
Conversation
Made acting_user a mandatory keyword argument by using (*, acting_user) in the functions. Passed acting user as a kwarg while calling these functions.
| @@ -862,7 +862,7 @@ def do_scrub_realm(realm: Realm, acting_user: Optional[UserProfile]=None) -> Non | |||
| event_type=RealmAuditLog.REALM_SCRUBBED) | |||
|
|
|||
| def do_deactivate_user(user_profile: UserProfile, | |||
| acting_user: Optional[UserProfile]=None, | |||
| *, acting_user: Optional[UserProfile]=None, | |||
There was a problem hiding this comment.
So, I think if we want to make this actually mandatory, we need to remove the =None. We can also remove the Optional[ in cases where we don't expect code to need to pass None.
There was a problem hiding this comment.
@timabbott Then I think we would have to pass acting_user=None in all the tests?. Should we have to record acting_user while running test cases?
There was a problem hiding this comment.
Hmm, well, we should be thoughtful. For do_deactivate_user in particular, probably it doesn't make sense to make this change because we have a lot of tests that need to deactivate a user. But for any of the do_ functions that are only called in at most 5 places total, we should make the change I suggest.
|
Heads up @arpit551, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
|
@arpit551 quick ping on updating this. |
timabbott
force-pushed
the
main
branch
2 times, most recently
from
4ec3636 to
88b200c
Compare
Made acting_user a mandatory keyword argument by using
(*, acting_user) in the functions.
Passed acting user as a kwarg while calling these functions.
Tested using ./tools/test-backend.