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: show non-restricted ads instead of not showing ads at all #213

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

arpitjalan
Copy link
Member

In some cases where there were category restricted house ads we were not showing ads on reload. This commit filter out all the ads that should not be shown on current page, leaving only allowed ads. So now we'll show ads on every reload in all the cases.

Internal ticket: t130920

In some cases where there were category restricted house ads we were not
showing ads on reload. This commit filter out all the ads that should
not be shown on current page, leaving only allowed ads. So now we'll
show ads on every reload in all the cases.

Internal ticket: t130920
Copy link
Member

@ZogStriP ZogStriP left a comment

Choose a reason for hiding this comment

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

LGTM. Made a comment about a potential edge case.

Copy link
Contributor

@janzenisaac janzenisaac left a comment

Choose a reason for hiding this comment

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

lgtm!

@arpitjalan arpitjalan merged commit 9b72130 into main Jun 27, 2024
5 checks passed
@arpitjalan arpitjalan deleted the housead-fix branch June 27, 2024 16:43
@nicolasblanco
Copy link

Hello, after this commit my users are experiencing JS errors on the House ad part, please find the stacktrace.

Screenshot 2024-07-04 at 15 02 17 Screenshot 2024-07-04 at 15 02 35

Thanks!

nicolasblanco added a commit to nicolasblanco/discourse-adplugin that referenced this pull request Jul 4, 2024
@arpitjalan
Copy link
Member Author

@nicolasblanco I have a PR up with the fix: #214

However, I am unable to repro this issue on my dev server. Do you have repro steps (exact settings to configure) for this issue so that I dig even deeper?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants