GitHub Sponsor path validation & tests #15608
Conversation
eviljeff
requested review from
a team and
bobsilverberg
and removed request for
a team
|
you've got some codestyle fails around line length https://travis-ci.org/github/mozilla/addons-server/jobs/731753477#L555 (you can replicate locally with |
There was a problem hiding this comment.
This looks great! Outstanding job, @conema! I had one suggestion for a possible improvement, but I'm seeking feedback from the primary server developers on that. Great work so far, and stay tuned!
| if not hostname.endswith(amo.VALID_CONTRIBUTION_DOMAINS): | ||
| raise forms.ValidationError(ugettext( | ||
| 'URL domain must be one of [%s], or a subdomain.' | ||
| ) % ', '.join(amo.VALID_CONTRIBUTION_DOMAINS)) | ||
| elif (hostname == 'github.com' and |
There was a problem hiding this comment.
I wonder if it's worth making github.com into a constant, so that it can be used here, in src/olympia/constants/base.py, and also in the tests, without having to hardcode it multiple times? It would also allow for it to be changed easily if for some reason the GitHub hostname changed.
There was a problem hiding this comment.
I don't think it's really worth the effort.
There was a problem hiding this comment.
Fair enough. This is good to land, then. Thanks for the patch @conema!
|
Thanks so much for the patch, @conema! 🙌 Your contribution has been added to our recognition wiki. Welcome onboard -- we look forward to seeing you around the project! 😎 |
Fixes mozilla/addons#7934
The PR implements a special-case path validator for GitHub Sponsor as described by @diox's reply and adds two tests.