Skip to content

feat: add API for related emails - #8671

Merged
rjsparks merged 5 commits into
ietf-tools:mainfrom
rpcross:email-api
Apr 16, 2025
Merged

feat: add API for related emails#8671
rjsparks merged 5 commits into
ietf-tools:mainfrom
rpcross:email-api

Conversation

@rpcross

@rpcross rpcross commented Mar 15, 2025

Copy link
Copy Markdown
Contributor

Add a API endpoint. Given one email address return all other related email addresses the Datatracker knows about

@codecov

codecov Bot commented Mar 15, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.79%. Comparing base (c70e67d) to head (25f6777).
Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
ietf/api/views.py 93.33% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jennifer-richards jennifer-richards left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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)

@jennifer-richards

Copy link
Copy Markdown
Member

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.

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 अजय@डाटा.भारत), as shown in the screen shot

image of browser address bar showing http://localhost:8000/person/अजय@डाटा.भारत

and added a debug.show("email_or_name") to the profile() view, which output

    email_or_name: 'अजय@डाटा.भारत'

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 [^/]+ that the profile() view uses, you'll need to work out a non-latin-aware regex. I suspect you can just leave it free and junk won't match any addresses, though.

@jennifer-richards jennifer-richards left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a small change needed to cleanly reject null characters, see inline. Probably should add a test case for that.

Comment thread ietf/api/views.py
@requires_api_token
@csrf_exempt
def related_email_list(request):
def related_email_list(request, email):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

@jennifer-richards jennifer-richards left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

@rjsparks
rjsparks merged commit 3d69b2c into ietf-tools:main Apr 16, 2025
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Apr 20, 2025
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.

3 participants