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

Auth/integration #190

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Auth/integration #190

wants to merge 3 commits into from

Conversation

pallabez
Copy link
Contributor

@pallabez pallabez commented Jan 6, 2024

Issue:

#191

Description:

Anything you would like to inform the reviewer about:

Dev Tested:

  • Yes
  • No

Put the Feature behind Feature Flag

  • Yes
  • No

Test Stats

Images/video of the change:

Follow-up Issues (if any):

NOTE: After your PR is merged in main please verify its working on prod and raise a dev to main PR

@pallabez pallabez self-assigned this Jan 6, 2024
Copy link

@ajoykumardas12 ajoykumardas12 left a comment

Choose a reason for hiding this comment

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

Reviewed with mostly JS knowledge and little Vue knowledge 😅
Also, any visuals for changes?

Comment on lines +42 to +48
user,
token,
setToken,
setUser,
signin,
signout,
isLoading,

Choose a reason for hiding this comment

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

Should token, setToken, setUser be exported? Or they are exported for tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the catch. Will address it.

Comment on lines +55 to +61
const LINKS = [
{ name: 'Welcome', link: 'https://welcome.realdevsquad.com' },
{ name: 'Events', link: 'https://www.realdevsquad.com/events.html' },
{ name: 'Members', link: 'https://members.realdevsquad.com/' },
{ name: 'Crypto', link: 'https://crypto.realdevsquad.com/' },
{ name: 'Status', link: 'https://status.realdevsquad.com/' }
]

Choose a reason for hiding this comment

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

These constants can be moved to constants file.

throw error // or Log here
})
.then(res => transformSelfInfoFromApi(res.data))

Choose a reason for hiding this comment

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

Not catching errors?

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