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

Rework integration of remote features for consistency between testing and live applications #8341

Closed
3 tasks
aaemnnosttv opened this issue Mar 1, 2024 · 6 comments
Labels
P1 Medium priority PHP Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature

Comments

@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented Mar 1, 2024

Feature Description

Remote features are currently set up unnecessarily late in the plugin bootstrapping which has led to at least one bug where a difference in behavior from our normal testing differed from pre-release testing with a remote-enabled feature. This can be addressed by setting up remote features as early as possible and breaking apart the bits which require running later.

See #8340 for a WIP POC.


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

Acceptance criteria

  • Remote feature infrastructure (and its minimal dependencies) should be initialized and registered as early as possible, but not outside of Plugin::register
  • The responsibilities of each part should be separated to allow for each to be set up as early as possible

Implementation Brief

  • Use Rework integration of remote features #8340 POC as starting point. Shortly it covers:
  • Update includes/Core/Util/Remote_Features.php:
    • Extend base Setting class, and keep only setting methods
    • Split the rest of the logic into classes:
      • includes/Core/Util/Remote_Features_Activation.php
        • Move googlesitekit_is_feature_enabled into the register method
        • Move here the get_features and filter_features methods
      • includes/Core/Util/Remote_Features_Sync.php
        • Move the cron logic here
  • Update includes/Plugin.php
    • Instantiate Remote_Features_Activation class in the register method and hook into the admin_init to instantiate the Remote_Features_Sync. Invoke register method and cron scheduling, as well as on_change method.
  • Wrap up the POC and fix failing tests

Test Coverage

  • tests/phpunit/integration/Core/Util/Remote_FeaturesTest.php should be updated, as it only handles the Setting related methods. You can check tests/phpunit/integration/Core/User_Input/Site_Specific_AnswersTest.php for an example for tests
    • Extract the existing tests into the appropriate new test files, so they go into the Remote_Features_ActivationTest and Remote_Features_SyncTest classes.
  • Update tests/phpunit/integration/PluginTest.php to verify the new changes on admin_init hook

QA Brief

  • This issue changes how remote features are applied. It would be good to test general plugin behavior (eg smoke testing, module setup, etc.) as well as feature-flag-specific behavior, both enabled by the Tester and "manually" as if received via the proxy in the DB. I believe @mohitwp has done this before but ask @aaemnnosttv if you're not sure.

Changelog entry

  • Ensure remote features are loaded as early as possible during plugin initialization.
@aaemnnosttv aaemnnosttv added P1 Medium priority Type: Enhancement Improvement of an existing feature PHP labels Mar 1, 2024
@aaemnnosttv aaemnnosttv self-assigned this Mar 1, 2024
@aaemnnosttv aaemnnosttv added the QA: Eng Requires specialized QA by an engineer label Mar 9, 2024
@ivonac4
Copy link
Collaborator

ivonac4 commented Mar 19, 2024

@aaemnnosttv are you still planning to work on this one? If not, please unassign yourself.

@zutigrm zutigrm self-assigned this Jun 20, 2024
@zutigrm
Copy link
Collaborator

zutigrm commented Jun 25, 2024

Unassigning myself as I am not familiar enough with this feature and the priorities it would need to be split into

@zutigrm zutigrm removed their assignment Jun 25, 2024
@aaemnnosttv aaemnnosttv added the Next Up Issues to prioritize for definition label Jun 26, 2024
@binnieshah binnieshah added the Team S Issues for Squad 1 label Jul 9, 2024
@zutigrm zutigrm assigned zutigrm and unassigned zutigrm Jul 10, 2024
@aaemnnosttv aaemnnosttv self-assigned this Jul 10, 2024
@aaemnnosttv aaemnnosttv removed the Next Up Issues to prioritize for definition label Jul 10, 2024
@aaemnnosttv aaemnnosttv removed their assignment Jul 29, 2024
@tofumatt tofumatt self-assigned this Jul 30, 2024
@tofumatt tofumatt removed the QA: Eng Requires specialized QA by an engineer label Jul 31, 2024
@tofumatt
Copy link
Collaborator

Removing QA:Eng since this QA should be a general plugin test with/without various feature flags enabled.

@tofumatt tofumatt removed their assignment Jul 31, 2024
@mohitwp mohitwp self-assigned this Aug 1, 2024
@mohitwp
Copy link
Collaborator

mohitwp commented Aug 2, 2024

@aaemnnosttv In QAB, I don’t understand the phrase Feature flag enabled 'manually as if received via the proxy in the DB.' Can you please explain what this means? Does it refer to setting up Site Kit using the developer plugin? Thanks !

@aaemnnosttv
Copy link
Collaborator Author

Oh sorry @mohitwp, I just meant the same behavior whether the feature was enabled via the Tester or set in the database (as it would be for a live site). Does that make more sense?

@mohitwp
Copy link
Collaborator

mohitwp commented Aug 7, 2024

QA Update ✅

  • Tested on main environment.
  • Smoke tested plugin.
  • Verified all modules setup.
  • Verified entity and main dashboard.
  • Verified settings page.
  • Verified Ad blocker functionality.
  • Verified Key metrics functionality.
  • Verified Enhanced measurement functionality.
  • Verified audience segmentation.
  • Verified modules when feature flag enabled by Tester plugin and or set in the database.
  • No console errors are found.
  • All functionalities working as expected.

@mohitwp mohitwp removed their assignment Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Medium priority PHP Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

6 participants