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

include Module_With_Deactivation Interface To the Ads module #8449

Closed
1 task done
zutigrm opened this issue Apr 1, 2024 · 3 comments
Closed
1 task done

include Module_With_Deactivation Interface To the Ads module #8449

zutigrm opened this issue Apr 1, 2024 · 3 comments
Labels
Good First Issue Good first issue for new engineers Module: Ads Google Ads module related issues P0 High priority Type: Enhancement Improvement of an existing feature

Comments

@zutigrm
Copy link
Collaborator

zutigrm commented Apr 1, 2024

Feature Description

Ads module is missing the implementation of Module_With_Deactivation interface.


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

Acceptance criteria

  • Ads module should implement Module_With_Deactivation interface and include on_deactivation method which should delete the module settings

Implementation Brief

  • Update Google\Site_Kit\Modules\Ads class
    • Implement the interface and method mentioned in AC
      • Delete the settings in on_deactivation method, eq $this->get_settings()->delete();

Test Coverage

  • Include test case for test_on_deactivation method to the tests/phpunit/integration/Modules/AdsTest.php

QA Brief

  • Enable adsModule feature flag
  • Connect Ads module, then disconnect it
  • Try to connect it again, verify that old value is not pre-filled

Changelog entry

  • N/A
@zutigrm zutigrm added Type: Enhancement Improvement of an existing feature Module: Ads Google Ads module related issues labels Apr 1, 2024
@zutigrm zutigrm assigned zutigrm and unassigned zutigrm Apr 1, 2024
@eugene-manuilov eugene-manuilov self-assigned this Apr 1, 2024
@eugene-manuilov
Copy link
Collaborator

AC ✔️

@eugene-manuilov eugene-manuilov removed their assignment Apr 1, 2024
@zutigrm zutigrm assigned zutigrm and unassigned zutigrm Apr 1, 2024
@eugene-manuilov eugene-manuilov self-assigned this Apr 1, 2024
@eugene-manuilov
Copy link
Collaborator

IB ✔️

@eugene-manuilov eugene-manuilov removed their assignment Apr 1, 2024
@zutigrm zutigrm self-assigned this Apr 1, 2024
@bethanylang bethanylang added the P0 High priority label Apr 1, 2024
@zutigrm zutigrm mentioned this issue Apr 1, 2024
18 tasks
@zutigrm zutigrm removed their assignment Apr 1, 2024
@wpdarren wpdarren added the Good First Issue Good first issue for new engineers label Apr 2, 2024
@kelvinballoo
Copy link
Collaborator

kelvinballoo commented Apr 2, 2024

QA Update ✅

Tests where done as follows:

  • Connected Ads module, then disconnected it successfully. Ensured that old value is not pre-filled for a new setup.
  • Test done on Dev branch on WP 6.4.2 PHP 8.0 mainly on these platforms:
    • Cross browsers MacOS Ventura (Firefox, Chrome, Safari)
    • Windows 11 Edge
    • Mobile (iOS,iPhone 15 Safari)
    • Mobile (Android Samsung S23 Chrome)
    • Tablet (iOS, iPad 10 Safari)
    • Tablet (Android, Galaxy Tab S7 )
  • Did one test on MacOS Ventura (Chrome) for WP 6.4.2 and PHP 7.4

Moving ticket to Approved column.

Screenshots

Able to successfully disconnect and the ads module appears under 'Connect More Services'

Screenshot 2024-04-02 at 17 35 48  

If we re-connect the ads module after disconnecting, the old value isn't pre-filled

Screenshot 2024-04-02 at 17 35 56

@kelvinballoo kelvinballoo removed their assignment Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Good first issue for new engineers Module: Ads Google Ads module related issues P0 High priority Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

6 participants