Skip to content

Sort sponsors #19551

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.

Sign up for GitHub

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

Closed
JacobCoffee wants to merge 3 commits into from
Closed

Sort sponsors #19551

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
52ee78d
sort the sponsors
JacobCoffee Feb 19, 2026
9d8763b
tests
JacobCoffee Feb 19, 2026
15ad28e
improve test
JacobCoffee Feb 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions tests/unit/sponsors/test_init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
import pretend

from celery.schedules import crontab
from sqlalchemy import true

from warehouse import sponsors
from warehouse.sponsors.models import Sponsor
from warehouse.sponsors.tasks import update_pypi_sponsors

from ...common.db.sponsors import SponsorFactory
Expand Down Expand Up @@ -47,11 +45,10 @@ def test_do_not_schedule_sponsor_api_integration_if_no_token():


def test_list_sponsors(db_request):
SponsorFactory.create_batch(5)
expected = SponsorFactory.create_batch(5)
SponsorFactory.create_batch(3, is_active=False)

result = sponsors._sponsors(db_request)
expected = db_request.db.query(Sponsor).filter(Sponsor.is_active == true()).all()

assert result == expected
assert len(result) == 5
assert set(result) == set(expected)
7 changes: 6 additions & 1 deletion warehouse/sponsors/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@


def _sponsors(request):
return request.db.query(Sponsor).filter(Sponsor.is_active == true()).all()
return (
request.db.query(Sponsor)
.filter(Sponsor.is_active == true())
.order_by(Sponsor.level_order, Sponsor.name)
.all()
)
Copy link
Copy Markdown
Member

@miketheman miketheman Feb 19, 2026

Choose a reason for hiding this comment

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

Setting this explicit order will impact anywhere we use request.sponsors and don't re-sort.

Copy link
Copy Markdown
Member Author

@JacobCoffee JacobCoffee Feb 19, 2026

Choose a reason for hiding this comment

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

i think its just sponsors.html and the sponsors footer (which i cant seem to see that we actually use/render...?) but this is the preferred thing so that we keep consistencies and sponsor expectations on ordering

Copy link
Copy Markdown
Member Author

@JacobCoffee JacobCoffee Feb 19, 2026

Choose a reason for hiding this comment

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

oh i have adblock so i missed these. that'd be a separate issue but i feel like we should do this in a way to not be blocked by ad-blockers

Copy link
Copy Markdown
Member

@miketheman miketheman Feb 19, 2026

Choose a reason for hiding this comment

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

I don't know if you're responding to this comment, or to the one in the HTML template - but it's unclear from the PR description on what the intent here is, so it's hard to review without the context.

It'd help if you viewed the current page without ad blockers, as well as the proposed changes so that you can see the differences. However, the sample sponsor data doesn't contain levels/order - I added that in #19553 so that might helpful to merge first and then see how this renders for you, and make the desired changes.



def includeme(config):
Expand Down
2 changes: 1 addition & 1 deletion warehouse/templates/pages/sponsors.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ <h2 class="centered-heading__title">Infrastructure sponsors</h2>
<hr class="centered-heading__rule">
</div>
<div class="sponsor-grid sponsor-grid--bottom-margin">
{% for sponsor in request.sponsors | sort(attribute="name") %}
{% for sponsor in request.sponsors %}
Copy link
Copy Markdown
Member

@miketheman miketheman Feb 19, 2026

Choose a reason for hiding this comment

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

This is only removing the name sort order from the infra sponsors, is that intentional? I suspect you'd want the "main" sponsors section instead / as well.

Copy link
Copy Markdown
Member Author

@JacobCoffee JacobCoffee Feb 19, 2026

Choose a reason for hiding this comment

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

i think this was preferable so that we ensure db-level sortnig for all instead of just infra

{% if sponsor.infra_sponsor %}
<div class="sponsor-grid__sponsor">
<div class="sponsor-grid__sponsor-img">{{ sponsor.color_logo_img|camoify|safe }}</div>
Expand Down
Loading