site-kit-wp: AdSense not being connected after going through the initial setup
Bug Description
AdSense not being connected after going through the initial setup This issue occurs once on fresh site. It occurs only one time while doing set up first time, after that this issue is not getting occur even after resetting the plugin.
Note : This issue is occurring when we try to do set up via clicking on Adsense CTA on dashboard or via settings page.
Steps to reproduce
- Create fresh site using dev plugin.
- Do initial set up.
- Try to set up AdSense through CTA on dashboard or via settings page.
- Check Dashboard and settings page will show AdSense not connected.
Screenshots
https://user-images.githubusercontent.com/1621608/179300930-e2337c4e-7270-406b-978e-a1081cafadc5.mp4
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- Under successful preconditions for connecting AdSense, the module’s setup should always complete successfully
Implementation Brief
We need to add a flag to prevent the AdSense background submitChanges() action being triggered while the user is actively submitting the changes.
- Use this draft PR as a guide.
- However, use a less abstract name for the flag. Rather than
isCompletingSetupetc., name the flagsuspendBackgroundSubmitand its permutations. - Review this comment for some tips on reproducing the issue.
Test Coverage
- No new tests needed.
QA Brief
The AdSense module should not be set up/configured prior to the QA steps that follow. If it is, reset the Site Kit plugin and perform the QA steps that follow:
- Perform set up steps for the Google AdSense module via the
Google Site Kit > Settingsscreen. - Follow the UI prompts / CTA’s to finalise set up of the AdSense service.
- Validate that the AdSense module is correctly set up and indicates as such in the
Google Site Kit > Settingsscreen within the connected modules section.
Give that it is not straightforward to replicate this race condition in all environments, the steps above (using a reset of the plugin each time) should be performed ~10 times.
Changelog entry
- Fix potential bug in AdSense set up which could leave the setup incomplete.
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Comments: 18
Sounds good, thanks. I was mostly thinking about something at the component level where we could force the race condition, by controlling when a request finishes, etc but it isn’t worth sinking a ton of time into.
IB ✅
Thanks @aaemnnosttv, I’ve updated the IB to use the naming that you’ve suggested, SGTM.
With regard to the testing though - if you are talking about E2E test coverage, I don’t really think it is possible to add coverage. It’s not that I’ve found a way to reproduce the issue on demand, simply that I have found that the user flow can be short circuited a bit.
The initial bug reports suggested that it only occurs on the first setup of a new site. In fact I’ve found that simply deleting the AdSense settings and then running through the setup again is enough, and this can be reduced (on an InstaWP site) to a few quick steps - pressing a button on the DB web interface to trigger the SQL deletion, and then navigating to a bookmark to the AdSense setup page to press the CTA.
However, the bug still only occurs about 1 in 5 times in my testing, which makes it unsuitable for an E2E test. Maybe there’s some way of setting up the right conditions e.g. by delaying a particular response but it’s not something I’ve come across during my somewhat lengthy investigation.
As for component/unit testing, it appears we currently don’t have any for the whole set of V2 AdSense components, so I don’t think it’s worthwhile introducing a complicated test purely for this edge case…
@bethanylang let’s prioritize this as soon as we can fit it in. It’s not severe enough to justify a P0 since the user isn’t blocked (they can just run through it again which is quick) but I feel like it has been happening more recently.