Simplify logic around user input completion state

[kuasha420]

Feature Description

Going forward, user input questions will no longer be mandatory to answer and the given answers will live on the sites WordPress database. Therefore, there will be only two states for a given user regarding User Input completion: complete and incomplete.

Also, it will be easier to infer this state from the saved values and can be represented as a boolean. This will significantly simplify logic around User_Input_State and User_Input_Settings.

Going forward, the User Input is optional for all users, therefore, there should no longer be any automatic redirection to user-input page based on the completion state of User Input Survey.

---

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

Acceptance criteria

• The User Input should no longer have a REQUIRED (mandatory) state.
• The User Input questions should be considered complete for a user when both of these conditions are met:
  • All the Site Specific questions are answered and
  • All the User specific questions are answered by the current user.
• The User_Input_State class should be removed and its usage should be refactored to make use of the above method.
• Users should not automatically be redirected from dashboard to user-input page when they have not completed the user input questions.

Implementation Brief

• In includes/Core/Util/User_Input_Settings.php:
  • Add a method that checks if all the user input questions have been answered, say (is_complete()). In this method:
    • Use the get() method from the same class (being implemented in Store User Input answers in Site Database #5898) to get the values of the user input questions.
    • Return true if the values contain answers for all three questions, i.e. purpose, postFrequency, and goals (being refined in Update questions and related copies of User Input Screens  #5888).
• In includes/Core/Authentication/Authentication.php:
  • Remove the following methods:
    • require_user_input()
    • verify_user_input_settings()
    • get_user_input_state()
  • In the register() method:
    • Remove the action (added to admin_init hook) that calls the verify_user_input_settings method.
    • Remove the action (added to admin_init hook) that redirects the user to the user-input page if User_Input_State is VALUE_REQUIRED.
    • Update the action added to the googlesitekit_authorize_user hook and remove the call the require_user_input() method (along with its condition checking if $previous_scopes is empty).
    • In the callback function of the filter added to the googlesitekit_user_data hook, replace the userInputState key added to the $user array with isUserInputComplete mapped to a call to the new is_complete() method from User_Input_Settings.
• Delete the User_Input_State class (i.e. delete includes/Core/Authentication/User_Input_State.php).
• In assets/js/googlesitekit/datastore/user/user-info.js:
  • Change the constant RECEIVE_USER_INPUT_STATE and its value to RECEIVE_IS_USER_INPUT_COMPLETE. Update all its usages accordingly.
  • Replace all instances of userInputState with isUserInputComplete
  • Change the name of the receiveUserInputState action to be receiveIsUserInputComplete. Update all its usages accordingly.
  • Change the getUserInputState selector to be getIsUserInputComplete. Update all its usages accordingly.
  • Change the getUserInputState resolver to be getIsUserInputComplete.
• Across the codebase:
  • Remove all usages of the User_Input_State class.
  • Remove all existing usages of the removed methods from includes/Core/Authentication/Authentication.php.
  • Replace all usages of the getUserInputState selector from the CORE_USER store:
    • If the name of the variable to which this selector is assigned to named userInputState, change it to isUserInputComplete.
    • Update the usages of the selector values so that it is treated as a boolean value instead of checking if it equals to complete.

Test Coverage

• Add test case for the new method (is_complete()).
• Remove test cases for the User_Input_State class (tests/phpunit/integration/Core/Authentication/User_Input_StateTest.php and others).
• Update test cases in the following suites to reflect all the above changes:
  • tests/e2e/plugins/user-input.php
  • tests/phpunit/integration/Core/Authentication/AuthenticationTest.php
  • tests/phpunit/integration/Core/Util/User_Input_SettingsTest.php
  • assets/js/googlesitekit/datastore/user/user-input-settings.test.js
  • assets/js/googlesitekit/datastore/user/user-info.test.js
• Fix any failing tests.

QA Brief

• With the userInput feature flag, verify the automatic redirect from dashboard to User Input Questionnaire does not happen. Only the new Key Metrics banner should be visible. Clicking on "Personalise your metrics" should take you to the questionnaire which should be fillable as usual.
• Verify the user input questionnaire is fillable normally from the Admin Settings as well.
• Since a substantial amount of code has been removed, including some from the Authentication class, smoke test the setup and authentication of the plugin and all modules.

Changelog entry

• Simplify User Input completion-related behavior.

[aaemnnosttv]

• The usage of User_Input_Settings should implement a simpler replacement for User_Input_State with only two state - complete (true) and incomplete (false).

@kuasha420 this is worded in a way that could be misinterpreted. I think the intention here is that User_Input_Settings should implement a new method for checking the completion status? Currently it reads as if the code that uses User_Input_Settings should change.

The usage of User_Input_State should be replaced with the above check.

• Both missing and required state should be replaced by incomplete state.

This part is a bit confusing as well. Above we're saying the completion status can be represented as a boolean now, but here we're referring to new named user input states. AFAIU, we should be able to remove User_Input_State entirely as part of this simplification. If that's the case then let's say that and then make it a bit more clear as to what should be done where it was used before.

[kuasha420]

@aaemnnosttv I have updated the wordings and merged #5901 with this one. It it looks good to you. feel free to close #5901. Cheers!

[aaemnnosttv]

Thanks @kuasha420 – LGTM 👍

[nfmohit]

Hi @kuasha420! 👋

Quick question regarding the ACs here. I’m working on the IB and it strikes me, do we really need a method to check the completion state of user input?

I ask this because since we no longer need to redirect the user to the user input page based on the completion state (we will only redirect once after authorisation, if user input settings are not set, which can be checked using User_Input_Settings::has being introduced in #5898), where would this method actually be used?

I'm sure I'm missing something and just in need of a little clarification.

Thank you!

[kuasha420]

Hi @nfmohit !

While brainstorming, my thought was that we will store the answers in separate Options field, that this function would return true if all the options are set ie. all questions answered. Which will be used in Key Metrics CTA in Dashboard and settings to determine whether the user has completed User Input questions. If we can easily determine that without this method, we can safely skip the method.

Hope that clears it up. Cheers.

[nfmohit]

Thank you for the clarification, @kuasha420!

[techanvil]

Hi @nfmohit, thanks for the IB.

I am not sure about this point:

• Update the action added to the googlesitekit_authorize_user hook so that if $previous_scopes is empty, it redirects the user to the user-input page instead of calling the require_user_input() method.

I might have misunderstood something here but this does not appear needed to fulfil the AC and seems counter to the stated aim of the User Input V2 Design Doc, which in the Overview states:

The User Input questions should not be asked immediately after Site Kit Setup for new users.

It looks to me like implementing the above point would result in new users being redirected to the User Input page.

Please could you take a look and either clarify or update/remove this point as necessary?

cc @kuasha420

[nfmohit]

Thank you for pointing that out, @techanvil. I have updated the IB to remove the condition and method call entirely.

[techanvil]

Thanks @nfmohit!

I've spotted something else, sorry for not realising yesterday. Bear in mind this AC point:

The User_Input_State class should be removed and its usage should be refactored to make use of the above method.

The IB removes User_Input_State and adds the is_complete method, but it doesn't do anything with is_complete. The front-end code that relies on userInputState will no longer work.

We should in fact, rather than simply removing userInputState from the inlined user data, be replacing it with something like isUserInputComplete => is_complete(), and updating the front-end code accordingly, to keep things working.

[wpdarren]

QA Update: ❌

I have an observation and a question.

1. After completing the questions and saving the Congrats! You set your site goals banner appears, and if you close that banner and then refresh the dashboard, the Key metrics banner reappears for a split second. On the screencast below if you pause it at 7 seconds, you'll see the banner. Note: when you navigate to the dashboard (rather than refreshing) the banner does not reappear from what I could see.

ui-3.mp4

2. Question: should the key metrics banner appear underneath the zero data state banner notification? I suspect so, but wanted to check because that is the current behaviour.

[jimmymadon]

@wpdarren Thanks for the thorough testing as always!

1. The Key metrics banner appearing in the issue above is still using the old User Input banner code which will completely be replaced in Scaffold Key metrics Setup CTA on Main Dashboard #6209 and Design Key metrics Setup CTA #6210. So we should tackle the flicker (if it still exists then) as part of those issues.
2. The KM banner/widget area should not appear when the site is in Gathering Data or Zero Data states. However, this is still not implemented. The gathering data check will be implemented as part of Scaffold Key metrics Setup CTA on Main Dashboard #6209 and the Zero Data state check in Hide Key Metrics banner/widget area when SC and GA are in a Zero Data State #6512.

[wpdarren]

QA Update: ✅

Thanks @jimmymadon for clarifying those issues will be fixed in other tickets.

Verified:

• Set up Site Kit with Analytics. Previously the user input questions automatically appeared on the dashboard but they no longer do. 👍 Instead, I see the Key metrics banner instead.
• When I click on the Personalize your metrics link I am redirected to the user input form.
• I didn't complete the questions but navigated back to the dashboard and the Key metrics banner still appears.
• When I complete the questions and submit the responses, I am redirected back to the dashboard and the Congrats! You set your site goals banner appears.
• When I go to Settings > Admin Settings, the Key metrics banner appears and when I click on the Personalize your metrics link I am redirected to the user input questions. I am able to complete and save the questions.
• Tested to make sure that the Key metrics banner does not appear when the site is in gathering state and/or does not have analytics connected.

ui-5.mp4

[felixarntz]

@aaemnnosttv @kuasha420 @jimmymadon Apologies, looking at this for approval, I'm lacking some context here. I have tried to find the pull request that addresses this issue but don't see it anywhere in the issue history, and somehow most references in here appear to be from December 2022 or older.

What is the context here? If this was implemented such a long time ago, why is it only in a release now? Or am I just missing something? :)

[jimmymadon]

@felixarntz Here is the PR that addresses this issue. Not sure why it didn't show up as a linked issue - I've linked it in Zenhub and via the comment template on the PR issue. Hope this is what you were looking for - thanks!

[aaemnnosttv]

I have tried to find the pull request that addresses this issue but don't see it anywhere in the issue history, and somehow most references in here appear to be from December 2022 or older.

@felixarntz the PR is linked above but that event was in a folded group by GitHub so you wouldn't see it without expanding that.

Note: I just amended the AC here to remove a reference to a specific class that no longer exists. There was already a relevant requirement, so that part was more of an implementation detail anyways.

This LGTM though so, closing it out!