Use EthicalAds to place logos under Sponsors page #1981
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
berinhard
added a commit
that referenced
this pull request
Feb 13, 2022
This change is also present at #1981, but since pypi integration also needs it, I decided to duplicate it to not have the integration between services pending on an open PR.
ewdurbin
pushed a commit
that referenced
this pull request
Feb 14, 2022
* Remove authentication that is already defined at base settings * Refactor tests to configure sponsorship applications during set up * Add publisher and logo place filters via querystring at logo placement API * Centralizes sponsor slug definition at pythondorg This change is also present at #1981, but since pypi integration also needs it, I decided to duplicate it to not have the integration between services pending on an open PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR replaces the current thumbnail solution to place sponsors logo by EthicalAds integration.
Here's a screen shot of the page working in my local environment.
One thing to notice is that the logo sizes will differ from production and my local environment. This is because the
ideal_dimensiontemplate filter depends on reading the image's width and height. And, even though I have a local of the sponsors app db, the same is not true for the logos. Thus, there was no way to calculate the ideal dimensions. I added a fallback for such scenario and it already improved the logo sizes, but I know that they will differ when this PR gets deployed.@ewdurbin another minor issue is the missing O'Reilly logo. This is because the logo file is formatted as EPS and probably EthicalAds had trouble processing such file format. I think it's better for us to convert the image to PNG and update our database instead of delegate the fix. Maybe this is also indicate we're missing some validation on the sponsorship application form to only accept JPG or PNG files as logos. What do you think about this?