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

Fixed: error when loading subscriptions without user signed in #2375

Conversation

malincrist
Copy link
Member

Problem

When initializing the "Subscriptions" widget, we check if the user is signed in and if not, we redirect them to the sign in page. However, the flow for loading subscriptions continues, even if there is no userId set, resulting in the error Unable to load subscriptions. Error: Parameter "userId" not specified.

Solution

If, after checking whether the user is signed in or not, we do not receive any userId in return, stop the execution of the initialization of the widget.

Copy link

Accessibility Insights Accessibility Insights Action: All applicable checks passed

  • URLs: 13 URL(s) passed, and 0 were not scannable
  • Rules: 29 check(s) passed, and 23 were not applicable
  • Download the Accessibility Insights artifact to view the detailed results of these checks

This scan used axe-core 4.3.2 with Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.4240.75 Safari/537.36.

Copy link
Collaborator

@rkolesnikovDX rkolesnikovDX left a comment

Choose a reason for hiding this comment

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

We have similar code in

Don't we need to have a similar check and throw readable error there?

@malincrist
Copy link
Member Author

We have similar code in

Don't we need to have a similar check and throw readable error there?

I don't think so, in the subscriptions widget, the issue is that we set this.userId = await this.usersService.ensureSignedIn(); and then rely on the userId to load the subscriptions.

In the profile widget we do this check just to redirect to sign in incase the user is not signed in, but we don't actually use the userId later for the initialization of the widget.
We use the userId only for specific actions triggered by the user. However the data for the user is taken from the next line of code const model: User = await this.usersService.getCurrentUser();, and this is based on the stored token. So, if the user is not signed in, there is no token, hence the user model is null. And since we have <!-- ko if: user --> in the html, we are safe because the buttons are not even displayed, so the user cannot trigger any action (super fast) before the redirect actually happens.

I don't think it's necessary to add it in the profile widget as well, except for maybe keeping consistency in the code. What do you think?

@malincrist malincrist merged commit 590ba34 into master Jan 25, 2024
10 checks passed
@malincrist malincrist deleted the malincrist/bug/error-when-loading-subscriptions-without-user-signedin branch January 25, 2024 16:40
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