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

(Re)Connecting Analytics hangs in loading state when Tag Manager Is connected #7937

Closed
zutigrm opened this issue Nov 30, 2023 · 4 comments
Closed
Labels
Module: Analytics Google Analytics module related issues Module: Tag Manager Google Tag Manager module related issues P1 Medium priority Type: Bug Something isn't working

Comments

@zutigrm
Copy link
Collaborator

zutigrm commented Nov 30, 2023

Bug Description

When Tag Manager module is connected, and Analytics disconnected and then, trying to connect again it hangs in the loading state.

There is no console error. When working on IB, check this comment as potential direction

Steps to reproduce

  1. Setup Site Kit
  2. Connect Analytics and Tag Manager modules
  3. Disconnect Analytics module
  4. Try to connect it again
  5. See setup being stuck in loading state

Screenshots

Screen.Recording.2023-11-30.at.13.09.51.mov

Additional Context

  • PHP Version:
  • OS: [e.g. iOS]
  • Browser: [e.g. chrome, safari]
  • Plugin Version: [e.g. 22]
  • Device: [e.g. iPhone6]

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

Acceptance criteria

  • A user should be able to connect or reconnect Analytics after Tag Manager has been set up.

Implementation Brief

  • So the issue is that we don't load getLiveContainerVersion anymore and so the loading checks for hasFinishedResolution for this selector returns false because the resolution never started (I confirmed this by calling hasStartedResolution). This is because we do not output the ExistingGTMPropertyNotice anymore, since we removed UA settings which did a property ID check, etc. So removing this check, as done in this PR on the SettingsEdit form is correct.
    • This should also be done on the new analytics-4 component that will be used soon too.
  • Since this is a very minor PR, I have gone ahead and attached it here with a QAB.

Test Coverage

  • No new tests required.

QA Brief

  • Setup Site Kit and connect GTM without connecting Analytics.
    • Verify that connecting Analytics works.
    • Verify that creating a new account works.
    • Verify that editing settings still works.

Changelog entry

  • Fix bug preventing Analytics from being setup when Tag Manager is connected.
@zutigrm zutigrm added the Type: Bug Something isn't working label Nov 30, 2023
@zutigrm zutigrm changed the title (Re)Connect Analytics Hangs in Loading State When Tag Manager Is Connected (Re)Connecting Analytics Hangs in Loading State When Tag Manager Is Connected Nov 30, 2023
@zutigrm
Copy link
Collaborator Author

zutigrm commented Nov 30, 2023

This is just a guess, but this might be regression from removing the ga4Reporting feature flag. We had similar issue, when editing Analytics settings with Tag Manager connected, edit settings would also hang in loading state. This was related to the gtmContainersResolved and it's hasFinishedLoadingGTMContainers check which was not used in GA4, you can check this PR, this might be similarly relating to this as well.

@techanvil techanvil added Module: Analytics Google Analytics module related issues Module: Tag Manager Google Tag Manager module related issues labels Dec 7, 2023
@mxbclang mxbclang changed the title (Re)Connecting Analytics Hangs in Loading State When Tag Manager Is Connected (Re)Connecting Analytics hangs in loading state when Tag Manager Is connected Dec 11, 2023
@aaemnnosttv aaemnnosttv added the P1 Medium priority label Feb 1, 2024
@jimmymadon jimmymadon self-assigned this Feb 13, 2024
@mxbclang mxbclang added the Next Up Issues to prioritize for definition label Feb 13, 2024
@jimmymadon jimmymadon removed their assignment Feb 13, 2024
@techanvil techanvil self-assigned this Feb 14, 2024
@techanvil
Copy link
Collaborator

techanvil commented Feb 14, 2024

Hi @jimmymadon, this IB looks good. One thing, though - with hasFinishedLoadingGTMContainers() no longer in use, we should remove that too (both versions of it).

Tbh, this can be a detail for the PR - I'll approve the IB ✅ and move this to Execution for an update.

@techanvil techanvil assigned jimmymadon and unassigned techanvil Feb 14, 2024
@mxbclang mxbclang removed the Next Up Issues to prioritize for definition label Feb 15, 2024
@jimmymadon
Copy link
Collaborator

Hi @jimmymadon, this IB looks good. One thing, though - with hasFinishedLoadingGTMContainers() no longer in use, we should remove that too (both versions of it).

Tbh, this can be a detail for the PR - I'll approve the IB ✅ and move this to Execution for an update.

Thanks @techanvil, I've replied there.

@techanvil techanvil assigned techanvil and unassigned jimmymadon and techanvil Feb 15, 2024
@mohitwp mohitwp self-assigned this Feb 15, 2024
@mohitwp
Copy link
Collaborator

mohitwp commented Feb 15, 2024

QA Update ✅

  • Tested on dev environment.
  • On latest environment able to reproduce the issue.
  • On dev environment issue is not reproducible and analytics is not getting stuck on reconnecting.
Recording.782.mp4

@mohitwp mohitwp removed their assignment Feb 15, 2024
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: Tag Manager Google Tag Manager module related issues P1 Medium priority Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants