feat: add API for related emails #8671
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8671 +/- ##
==========================================
- Coverage 88.81% 88.79% -0.02%
==========================================
Files 314 314
Lines 41255 41468 +213
==========================================
+ Hits 36642 36823 +181
- Misses 4613 4645 +32 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Rather than using querystring parameters to identify the email, I think it'd be better to use a URL parameter. E.g., /api/person/email/joe@example.com/related/ instead of /api/person/email/related?email=joe@example.com.
We should also make sure that this can handle non-latin characters (e.g., अजय@डाटा.भारत, borrowing a Hindi example from https://en.wikipedia.org/wiki/International_email). This may require explicit use of urllib.parse.quote and unquote but I'm not certain. Django may do some magic there. The tests should exercise this.
(Datatracker itself may have issues with internationalized email addresses, not sure offhand, but we should make sure that this API will work with them if they exist)
Good news here - it looks like Django sorts this out cleanly. Using Firefox, I requested the profile of a (non-existent) Person with the email and added a This shows that Django is decoding the URL string correctly and automatically. The regular expressions we use to match URL parameters in some places, however, don't work with non-ascii strings. If you need to constrain it beyond the simple |
| @@ -547,28 +547,24 @@ def active_email_list(request): | |||
|
|
|||
| @requires_api_token | |||
| @csrf_exempt | |||
| def related_email_list(request): | |||
| def related_email_list(request, email): | |||
There was a problem hiding this comment.
This needs a guard against null characters in the email, along the lines of
if "\x00" in emai:
return HttpResponseBadRequest()(or perhaps equivalent using _http_err()). We also need this in some other places in the existing code.
To see the issue, request something with a %00 in the parameter. Note that going to http://localhost:8000/ as we do for normal testing can hide the problem because the nginx proxy in front of datatracker generates its own 400. To get around that, open a shell in the app container and use something like curl 'http://localhost:8001/person/email/bad%00address/related/' -v -o /dev/null. It should return a 400, not a 500 error.
There was a problem hiding this comment.
Looks good. The tests on main will be fixed soon, so once #8804 is merged we can merge that here and let the tests run.
Add a API endpoint. Given one email address return all other related email addresses the Datatracker knows about