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

Some components remain in loading state on error #4542

Closed
mitogh opened this issue Dec 10, 2021 · 7 comments
Closed

Some components remain in loading state on error #4542

mitogh opened this issue Dec 10, 2021 · 7 comments
Labels
Module: Analytics Google Analytics module related issues Module: Search Console Search Console module related issues P1 Medium priority Rollover Issues which role over to the next sprint Type: Bug Something isn't working

Comments

@mitogh
Copy link
Contributor

mitogh commented Dec 10, 2021

Bug Description

Given a user that is authorized to a module (Analytics), after the user is granted the appropriate permissions and the user connects both modules if the user no longer has access to the data of those modules (the user permissions were removed from Analytics or Search Console the module never terminates the loading process and remains to load when visiting the modules section.

Steps to reproduce

  1. First assign a user to a property in Google Analytics.
  2. Then connect your Site Kit Plugin with this assigned user,
  3. Then connect the Google Analytics module
  4. Observe the module working as expected.
  5. Then remove this user access from the Google Analytics page so this user no longer has access to this report.
  6. Then from your browser visiting the Site Kit dashboard remove all the Session Storage values by going in Settings > More Tools > Developer Tools. Click on Application > Session Storage and remove all the values from the site kit
  7. Refresh the page and observe the page showing the loading indicator every time you refresh the page.

Screenshots

Analytics
2021-12-10_14-52

Search Console

2021-12-10_14-52_1

Main Dashboard

2021-12-10_15-40


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • Components should render their respective error states rather than a loading state when a report request results in an error

Implementation Brief

  • Update isGatheringData and hasZeroData selectors such that:
    • If the underlying getReport has not yet finished resolution, return undefined
    • If the underlying getReport has finished resolution and the report is not an array return false – an error occurred while fetching the report
  • Update and/or remove instances where the result of getReport(...) is used to infer a loading state
    • If a value based on hasFinishedResolution is available, remove the comparison based on the report value
    • If a condition is only based on the report value, update it to use a value based on hasFinishedResolution instead
  • Finish and merge Fix loading states when report errors #5000
    • PR only includes updates to isGatheringData so far
  • Ensure Storybook cases are correct

Test Coverage

  • No changes

QA Brief

  • Follow the steps from the steps to reproduce.
  • The widgets will no longer be stuck on loading state, they will load the error state of the widgets.

Changelog entry

  • Fix infinite loading state for components relying on gathering or zero data reports.
@mitogh mitogh added the Type: Bug Something isn't working label Dec 10, 2021
@mitogh mitogh changed the title When authorization is removed the widget keeps loading When user authorization is removed the widget keeps loading Dec 10, 2021
@aaemnnosttv
Copy link
Collaborator

@wpdarren would you please give this a test to see if you're able to reproduce it?

@wpdarren
Copy link
Collaborator

wpdarren commented Feb 1, 2022

@aaemnnosttv yes, I was able to reproduce this.

Rather than seeing the User does not have sufficient permissions message, the flashing loading indicator appears on the Analytics module page and the current and unified dashboard.

Screenshots

image

image

@wpdarren wpdarren assigned aaemnnosttv and unassigned wpdarren Feb 1, 2022
@aaemnnosttv aaemnnosttv added the P1 Medium priority label Feb 2, 2022
@aaemnnosttv aaemnnosttv changed the title When user authorization is removed the widget keeps loading Some components remain in loading state on error Mar 29, 2022
@aaemnnosttv aaemnnosttv added Module: Analytics Google Analytics module related issues Module: Search Console Search Console module related issues labels Mar 29, 2022
@aaemnnosttv aaemnnosttv removed their assignment Mar 29, 2022
@eugene-manuilov eugene-manuilov self-assigned this Mar 30, 2022
@eugene-manuilov
Copy link
Collaborator

IBR ✔️

@eugene-manuilov eugene-manuilov removed their assignment Mar 30, 2022
@aaemnnosttv
Copy link
Collaborator

@eugene-manuilov I just realized the suggested changes also apply to our hasZeroData selectors which are very similar in their implementation so I've updated the IB to include these as well and bumped the estimate accordingly – does that sound okay to you?

@eugene-manuilov
Copy link
Collaborator

Yes, looks good to me, @aaemnnosttv. Thanks for adding it! IB ✔️

@aaemnnosttv
Copy link
Collaborator

Moving this to the next release due to some unforeseen side effects that have come up as a result of these changes.

@kuasha420 kuasha420 assigned aaemnnosttv and unassigned kuasha420 Apr 22, 2022
@FlicHollis FlicHollis added the Rollover Issues which role over to the next sprint label Apr 25, 2022
@aaemnnosttv aaemnnosttv assigned kuasha420 and unassigned aaemnnosttv Apr 25, 2022
@kuasha420 kuasha420 assigned aaemnnosttv and unassigned kuasha420 Apr 27, 2022
@aaemnnosttv aaemnnosttv assigned kuasha420 and unassigned aaemnnosttv Apr 27, 2022
@kuasha420 kuasha420 removed their assignment Apr 27, 2022
@tofumatt tofumatt assigned tofumatt and unassigned tofumatt Apr 28, 2022
@wpdarren wpdarren self-assigned this Apr 29, 2022
@wpdarren
Copy link
Collaborator

QA Update: ✅

Verified:

  • Could reproduce the error on the latest release.
  • When switched to develop branch, I could see that the widgets will no longer be stuck on loading state and they load the error state of the widgets.

image

@wpdarren wpdarren removed their assignment Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Analytics Google Analytics module related issues Module: Search Console Search Console module related issues P1 Medium priority Rollover Issues which role over to the next sprint Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants