Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[24.0] Return generic message for password reset email #18479

Merged
merged 5 commits into from
Jul 5, 2024
Merged

[24.0] Return generic message for password reset email #18479

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4f5f8de
[24.0] Return generic message for password reset email
ahmedhamidawan Jul 1, 2024
a6e7081
adjust `test_reset_email_user_deleted` for `None` instead of str
ahmedhamidawan Jul 1, 2024
9727b88
remove else block and return `None` by the outer conditional
ahmedhamidawan Jul 1, 2024
077832b
Update test/unit/app/managers/test_UserManager.py
ahmedhamidawan Jul 1, 2024
839fd0c
Update lib/galaxy/managers/users.py
ahmedhamidawan Jul 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/galaxy/managers/users.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ def send_reset_email(self, trans, payload, **kwd):
log.debug(body)
return f"Failed to submit email. Please contact the administrator: {util.unicodify(e)}"
else:
return "Failed to produce password reset token. User not found."
return None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer need the else clause: None will be returned by the outer conditional. To simplify, you could just have return None as the last line in the method's body, outside the conditionals' scope.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or just drop the else: clause completely, as returning None is the default.

Copy link
Member

@jdavcs jdavcs Jul 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but ( I think) mypy will fail with a missing return statement. The argument, I think, goes like this: if there is no previous return statement, it's OK to let it fall through to the default None; but if there is a return statement in a previous clause, adding an explicit return None improves readability. (I don't remember the source, but I do support the argument)


def get_reset_token(self, trans, email):
reset_user = get_user_by_email(trans.sa_session, email, self.app.model.User)
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/webapps/galaxy/controllers/user.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ def reset_password(self, trans, payload=None, **kwd):
payload = payload or {}
if message := self.user_manager.send_reset_email(trans, payload):
return self.message_exception(trans, message)
return {"message": "Reset link has been sent to your email."}
return {"message": "If an account exists for this email address a confirmation email will be dispatched."}

def __get_redirect_url(self, redirect):
if not redirect or redirect == "None":
Expand Down
Loading