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

BB-718 CritiqueBrainz auth endpoint is called server-side #1041

Merged
merged 2 commits into from
Jan 12, 2024

Conversation

kirtanchandak
Copy link
Contributor

Problem

BB-718: CritiqueBrainz auth endpoint is called server-side

Solution

The conditional check - typeof window !== "undefined" ensures that the getAccessToken method is only called when the code is executed in a browser environment (client-side), preventing the unnecessary call to the authentication endpoint on the server-side.

Areas of Impact

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

@kirtanchandak kirtanchandak changed the title auth endpoint fixed on client side CritiqueBrainz auth endpoint is called server-side Dec 18, 2023
@kirtanchandak kirtanchandak changed the title CritiqueBrainz auth endpoint is called server-side BB-718 CritiqueBrainz auth endpoint is called server-side Dec 18, 2023
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.

Thanks for submitting a PR @kirtanchandak
Sorry it took so long for me to get to it!

That should do the trick!
Could you please add a comment above with a short explanation of why we are doing this check?
For future developers to know what's going on.

@MonkeyDo
Copy link
Member

In the future, please assign the ticket to yourself in Jira if you are working on one, to avoid someone else working on it at the same time.

@kellnerd
Copy link
Contributor

For what it's worth, the ticket was assigned to @kirtanchandak at the time they submitted the PR. It's the fault of the other contributor which reassigned the ticket to themselves later without asking and ignoring the link to this PR.

@MonkeyDo
Copy link
Member

Ah, my bad, I totally read that wrong! Thank you @kellnerd for correcting me and thank you @kirtanchandak for following the rules properly :)
Sorry for accusing you !

@kirtanchandak
Copy link
Contributor Author

@MonkeyDo no worries for the misunderstanding.
I've added the comment.

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.

Thank you ! 🚀

@MonkeyDo MonkeyDo merged commit c470250 into metabrainz:master Jan 12, 2024
4 checks passed
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.

3 participants