Skip to content
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.

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

fix: globally pinned items rank in global #1814

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Soxasora
Copy link
Member

@Soxasora Soxasora commented Jan 12, 2025

Description

Excludes items that have a pinId but no subName (global items) from the items query if it's ran in home, fixing #1736

Screenshots

before:
image

after:
image

Additional Context

n/a

Checklist

Are your changes backwards compatible? Please answer below:
Yes

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
10, array from query doesn't contain items that have a pinId and no subName, pins are later injected correctly by itemsWithPins

For frontend changes: Tested on mobile, light and dark mode? Please answer below:
n/a

Did you introduce any new environment variables? If so, call them out explicitly here:
No

Copy link
Member

@huumn huumn left a comment

Choose a reason for hiding this comment

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

As is, this will prevent an item that's pinned in another territory from ranking in hot on the homepage won't it? People got really upset when we did that before.

I suspect we'll want this to be done in the SQL query. We probably already have the correct behavior for pins in territories, but it doesn't work when sub = null, ie when it's the homepage.

@Soxasora Soxasora force-pushed the fix_duplicate_pin_items branch from cbc0ef4 to ca17b67 Compare January 12, 2025 23:29
@Soxasora
Copy link
Member Author

Soxasora commented Jan 12, 2025

I suspect we'll want this to be done in the SQL query.

Way better! Didn't think of going deeper than itemsWithPins. I edited the main post accordingly ^^

@Soxasora Soxasora added the bug label Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants