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: 404 instead of 300 for ambiguous email_or_person #8004

Merged
merged 4 commits into from
Oct 16, 2024

Conversation

jennifer-richards
Copy link
Member

@jennifer-richards jennifer-richards commented Oct 4, 2024

This changes both the CommunityList family of views and the Person photo view to return a 404 instead of a 300 if the email_or_name in the URL is ambiguous. In both cases, the old approach was attempting to be helpful but assumed that every Person matched had a User whose username could be used as a unique identifier. That is not true, so something needs to be done.

While the 300 response with a list of usernames (or email addresses) is arguably helpful and friendly, I don't think a 404 is wrong - we're just declaring that we don't have a CommunityList or Photo that can be found at that URL. In either context, someone who is bothered by the missing resource is likely to complain about it and we can address the underlying issue.

Fixes #8003

Copy link

codecov bot commented Oct 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.91%. Comparing base (c7f6bde) to head (a1f2705).
Report is 103 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8004      +/-   ##
==========================================
+ Coverage   88.78%   88.91%   +0.12%     
==========================================
  Files         296      304       +8     
  Lines       41320    41279      -41     
==========================================
+ Hits        36687    36703      +16     
+ Misses       4633     4576      -57     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rjsparks
Copy link
Member

The line exposing username was incorrect anyhow.
I suspect we'll end up needing to revisit this, likely with person search.

@rjsparks rjsparks merged commit a338df1 into ietf-tools:main Oct 16, 2024
9 checks passed
@jennifer-richards jennifer-richards deleted the multiple-person-error branch October 16, 2024 16:50
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unhandled exception / server error in lookup_community_list() when raising a MultiplePersonError when Person does not have a user
2 participants