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.

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

Fix exception serialization in user deletion #8450

Merged
merged 2 commits into from
Jan 23, 2024
Merged

Conversation

marcospri
Copy link
Member

@marcospri marcospri commented Jan 23, 2024

Fixes: https://hypothesis.sentry.io/issues/4905837844/?project=37293&referrer=github-pr-bot

Took the opportunity to refactor some tests around this file.

@@ -144,8 +144,8 @@ def users_delete(request):


@view_config(context=UserNotFoundError)
def user_not_found(exc, request): # pragma: no cover
request.session.flash(Markup(_(exc.message)), "error")
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only non-test code change in the PR.

@@ -75,19 +76,6 @@ def test_users_index_strips_spaces(models, pyramid_request):
models.User.get_by_username.assert_called_with(pyramid_request.db, "bob", "foo.org")


@users_index_fixtures
Copy link
Member Author

Choose a reason for hiding this comment

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

Merged some tests in this file, they were one-assertion-per-test style which becomes verbose pretty quickly. Done in a separate commit.

@@ -204,6 +153,13 @@ def test_users_delete_user_not_found_error(user_service, pyramid_request):
users_delete(pyramid_request)


def test_user_not_found_view(pyramid_request):
Copy link
Member Author

Choose a reason for hiding this comment

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

New test for previously un-tested view.

@marcospri marcospri force-pushed the fix-exception-message branch from cff1e02 to 2ec6058 Compare January 23, 2024 09:49
@marcospri marcospri requested a review from seanh January 23, 2024 09:52
Base automatically changed from remove-reindex to main January 23, 2024 13:47
@marcospri marcospri force-pushed the fix-exception-message branch from 2ec6058 to a116675 Compare January 23, 2024 13:54
@marcospri marcospri merged commit 62108d1 into main Jan 23, 2024
9 checks passed
@marcospri marcospri deleted the fix-exception-message branch January 23, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants