site-kit-wp icon indicating copy to clipboard operation
site-kit-wp copied to clipboard

Extract a new REST controller from core `Authentication`

Open aaemnnosttv opened this issue 1 year ago • 1 comments

Feature Description

Similar to the work we did in #5732, this issue is about extracting the REST related behavior into a new controller.


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

Acceptance criteria

  • All rest routes registered and related helper methods and defined by the core Authentication class should be extracted to a new Google\Site_Kit\Core\Authentication\REST_Authentication_Controller class
    • An accompanying test case should be created for it, similar to others that exercises the REST endpoints it registers and all REST-related tests should move from AuthenticationTest to the controller's test case
    • Test coverage should be added for any untested endpoints
    • The controller should be registered within Authentication::register
  • No change in behavior should be made/observable here

Implementation Brief

  • [ ] Add Google\Site_Kit\Core\Authentication\REST_Authentication_Controller class
    • In construct method pass the Authentication instance, and set it as a class property so all the needed methods can be accessed easily.
    • Add register method, and move googlesitekit_apifetch_preload_paths and googlesitekit_rest_routes filters from Authentication class here
    • Move get_rest_routes method from Authentication class, and use it's instance from class property to access the needed methods there
  • [ ] Update includes/Core/Authentication/Authentication.php
    • In construct, instantiate REST_Authentication_Controller class, passing $this, and invoke REST_Authentication_Controller::register method in the Authentication::register

Test Coverage

  • Add REST_Authentication_ControllerTest.php
    • Add tests for googlesitekit_rest_routes and googlesitekit_apifetch_preload_paths
    • Move over the test_disconnect
    • Add test coverage for connection and authentication endpoints, confirming the data returned, and get-token, you can save it using OAuth_Client::OPTION_ACCESS_TOKEN

QA Brief

  • Set up Site Kit on a new site
  • Set up all modules and uninstall all modules

No errors should occur during authentication.

Changelog entry

  • N/A

aaemnnosttv avatar Jun 05 '24 21:06 aaemnnosttv

IB ✔️

eugene-manuilov avatar Jun 20 '24 16:06 eugene-manuilov

QA Update ✅

  • Tested on dev envrironment.
  • Done smoke testing.
  • Verified all modules setup successfully.
  • Verified all modules disconnected successfully.
  • No console errors or dashboard errors found.

mohitwp avatar Jul 08 '24 05:07 mohitwp

@10upsimon I left a comment on the PR but there is a little bit of follow-up to do here. This is good to go for now, but let's open another issue to address the rest. Let me know if you have any questions, thanks!

aaemnnosttv avatar Jul 12 '24 19:07 aaemnnosttv