site-kit-wp
site-kit-wp copied to clipboard
Store User Input answers in Site Database
Feature Description
Currently, the answers to the User Input questions are being stored in the Site Kit Service and then being synced and cached. Going forward, the answers should be stored on the WordPress Site's Database.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- Answers to both user specific and site specific User Input questions must be persistently saved in the WordPress database.
- The related REST route should be updated to retrieve and store User Input answers from the database.
Implementation Brief
In includes/Core/Util/User_Input_Settings.php
, add the following changes:
- Extend the
User_Input_Settings
withUser_Setting
abstract class. - Create a new constant
OPTION
with valuegooglesitekit_user_input_settings
. - In
REST:get_routes
method update the following changes in theuser-input-settings
REST route:- Use the
User_Input_Settings::get
method to retrieve the user input settings instead of theget_settings
method. - Use the
User_Input_Settings::set
method to update the user input settings instead of theset_settings
method.
- Use the
- Update any other references to the
get_settings
andset_settings
methods to use theUser_Input_Settings::get
andUser_Input_Settings::set
methods respectively. - Update the
Authentication::verify_user_input_settings
method with the following changes:- Use the
User_Input_Settings::get
method to retrieve the user input settings instead of theUser_Input_State::get
method. - Use the
User_Input_Settings::has
method to check if the user input settings are set instead of theUser_Input_Settings::are_settings_empty
method.
- Use the
- Removing the legacy code (unused properties and methods) will be implemented in #5899.
Test Coverage
- Fix any failing tests.
- Update the tests for the above changes.
QA Brief
Changelog entry
Update the
User_Input_Settings::get_settings
with the following logic:
- Remove the existing logic.
- Retrieve the settings from the
parent::get
method.- Return the settings if they are not empty otherwise, return the default value from the
get_default
method.Update the
User_Input_Settings::set_settings
with the following logic:
- Remove the existing logic.
- Get the current settings from the
get_settings
method and merge the new settings with the current settings.- Set the merged settings using the
set
method, inherited from theUser_Setting
class.
@hussain-t this is not needed because the parent class User_Setting
already implements get
and set
methods that do everything what we need. We just need to remove these methods and update the user input settings usage to use get
and set
methods instead of get_settings
and set_settings
.
The are_settings_empty
method should be update to be the has
method. The usage of the are_settings_empty
method should be updated accordingly.
Thanks, @hussain-t. IB ✔️
@hussain-t Quick question for a little confirmation, so that I don't spend time in the wrong direction. Do you think it'd be wise not to store the site
specific questions in User_Setting
and store it in Setting
instead? If so, we could then re-purpose the User_Input_Settings
class to be a wrapper above underlying classes for user specific questions and site specific questions. What do you think?
CC: @eugene-manuilov
@nfmohit, it wasn’t mentioned where to store data in the User Input V2 docs. However, based on this point, your suggestion could be right:
The check for whether a user has completed the questionnaire can be determined by checking the associated
Setting
andUser_Setting
.
@kuasha420, WDYT?
@hussain-t That is correct and was the intention. Nice.
cc @nfmohit
@kuasha420 please assign to me for MR once you've completed your review. Thanks!
QA:Eng :white_check_mark:
- Went through the User Input flow and verified the settings were correctly saved in the DB:
QA Update: ❌
@nfmohit I have a few observations, which might be expected but would like to check.
-
When I have more than one admin, the message
Your answers will apply to the entire WordPress site: any other admins with access to Site Kit can see them and edit them.
appears on the first question on load. When I click one of the radio buttons to answer the question, the message disappears. Is this expected? It's a weird UX/UI experience in my opinion. -
The notice above also only appearing on the first question. I am assuming is expected?
-
If Google Analytics is not connected, and I submit the user input questions, when I am redirected to the Site Kit Dashboard, an error appears in the console. Is this expected?
Google Site Kit API Error method:GET datapoint:report type:modules identifier:analytics error:"Module must be active to request data."
- As Admin 1, I answer all the questions and submit them. They appear in the admin settings as expected. I then login as Admin 2 and connect Site Kit with a different Google account. I am redirected to the the user input questions, but I do not see the message
This question has been answered by:
and the first question isn't selected on Admin 1 response by default. Is this expected at this stage?
This leads me to that at the moment, when you login with different users, using different Google accounts, each user is able to submit their own separate answers, is this expected too?
Thank you for sharing your findings, @wpdarren.
- When I have more than one admin, the message
Your answers will apply to the entire WordPress site: any other admins with access to Site Kit can see them and edit them.
appears on the first question on load. When I click one of the radio buttons to answer the question, the message disappears. Is this expected? It's a weird UX/UI experience in my opinion.
That is very odd and I can't seem to be able to replicate it. Here's a screencast:
- The notice above also only appearing on the first question. I am assuming is expected?
That is expected. Only the first question is a site-scoped question. Answers to the other questions vary and are stored separately for each user.
3. If Google Analytics is not connected, and I submit the user input questions, when I am redirected to the Site Kit Dashboard, an error appears in the console. Is this expected?
Google Site Kit API Error method:GET datapoint:report type:modules identifier:analytics error:"Module must be active to request data."
Interesting. I was able to replicate this. However, upon further investigation of the code, it looks like this is happening due to the changes made in #5895. Could we raise it as a potential regression there?
7. As Admin 1, I answer all the questions and submit them. They appear in the admin settings as expected. I then login as Admin 2 and connect Site Kit with a different Google account. I am redirected to the the user input questions, but I do not see the message
This question has been answered by:
and the first question isn't selected on Admin 1 response by default. Is this expected at this stage?This leads me to that at the moment, when you login with different users, using different Google accounts, each user is able to submit their own separate answers, is this expected too?
This is expected at this stage. This happens due to my last point in the QA Brief:
Note: Currently, the user input answers will be wiped out if a user signs into Site Kit. This should be resolved once https://github.com/google/site-kit-wp/issues/5900 is merged.
QA Update: ⚠️
@nfmohit for point 1, on a fresh new site (InstaWP and TasteWP) I am still seeing the issue. Would you be able to try and recreate it on a new site, rather than on your local. Have been able to recreate this a few times.
https://user-images.githubusercontent.com/73545194/207268972-b6ab225e-4616-45bd-ab23-026142015075.mp4
Thank you for the re-confirmation, @wpdarren.
I was able to replicate this. Apparently, this only happens when the user has not completed the user input flow yet, which is why I wasn't able to replicate it as I had done it once and was updating it to try and replicate.
For more technical context, when the user input is not at a completed
state, the user responses are stored in the cache (session storage). When storing the cache, only the answers were saved, not the scope
. This makes the UI lose the scope
state and turn it to undefined
, causing the reported unexpected behaviour.
Just to clarify, this isn't a regression from this particular issue. I believe this existed from the initiation.
I have opened #6321 to address this.
QA Update: ✅
Verified:
- Went through the User Input flow with different admins multiple times and the completed answers are correctly displayed after completion.
- A second admin sees the site scoped question (currently only the Purpose question) as answered.
- There are no network request failures, console errors, or debug.log errors.
- Confirmed that the issue reported here has been fixed. When going through the user input flow for the first time for a user, verify that this issue no longer exists.
Currently, the user input answers will be wiped out if a user signs into Site Kit. This should be resolved once https://github.com/google/site-kit-wp/issues/5900 is merged.
Screenshots