Conversation
miketheman
left a comment
miketheman
left a comment
There was a problem hiding this comment.
How does this relate with the existing logic where the level and order are set by upstream?
|
if i understnad what you're asking @miketheman the query uses what the task pulls in , i don't think it was being used until this pr |
| .filter(Sponsor.is_active == true()) | ||
| .order_by(Sponsor.level_order, Sponsor.name) | ||
| .all() | ||
| ) |
There was a problem hiding this comment.
Setting this explicit order will impact anywhere we use request.sponsors and don't re-sort.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
| </div> | ||
| <div class="sponsor-grid sponsor-grid--bottom-margin"> | ||
| {% for sponsor in request.sponsors | sort(attribute="name") %} | ||
| {% for sponsor in request.sponsors %} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
i think this was preferable so that we ensure db-level sortnig for all instead of just infra
2 participants
Sort by level and then name