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 BB-718:CritiqueBrainz auth endpoint is called server-side #1044

Closed
wants to merge 1 commit into from

Conversation

AVtheking
Copy link

Problem
https://tickets.metabrainz.org/browse/BB-718

Solution
I have applied a conditional check (typeof window!=="undefined") this check ensures that this function is called from the client side .

Areas of Impact

The changes were reflected in src/client/components/pages/entities/cbReviewModal.tsx

@AVtheking
Copy link
Author

please review this

Copy link
Member

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

Hello @AVtheking and thanks for submitting a PR !

Unfortunately, a similar PR was submitted prior to yours in #1041 and I will merge that one.
The solution is the same, so good work on that :)

However for future reference, you'll see that there are a lot of unrelated formatting changes committed as well which we don't want.
I assume your editor is set up to automatically format code with Prettier or some other formatter, but we use ESLint to do so (see the eslint config at the root)

@MonkeyDo
Copy link
Member

Closing in favor of #1041

@MonkeyDo MonkeyDo closed this Jan 12, 2024
@MonkeyDo
Copy link
Member

Actually @AVtheking I was just corrected and pointed out that you had reassigned the ticket to yourself even though it was already assigned to someone. Please don't do that it's bad form and creates confusion.

@AVtheking
Copy link
Author

Ok sir

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

Successfully merging this pull request may close these issues.

2 participants