dev: Refactor Algolia client wrapper#82
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the Algolia client wrapper by relocating it from Inc\Algolia to Modules\Search, consolidating credential management into a new REST controller, and removing the admin_key field throughout the codebase.
Key changes:
- Creates a new
Algoliaclass inModules\Searchwith improved encapsulation and index caching - Introduces
Search_Controllerfor handling Algolia credential GET/POST operations - Removes client-side credential caching and the now-unused
admin_keyfield - Updates all references across the codebase to use the new namespace
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
inc/Modules/Search/Algolia.php |
New Algolia wrapper with private methods, index caching, and credential fetching for both governing and consumer sites |
inc/Modules/Rest/Search_Controller.php |
New REST controller for algolia-credentials endpoint with simplified validation |
inc/Modules/Search/Settings.php |
Removes admin_key references from credential storage and retrieval |
inc/Modules/Rest/Governing_Data.php |
Removes credential fetching and caching logic (moved to Algolia class) |
inc/classes/rest/class-basic-options.php |
Removes old credential endpoints and validation logic |
inc/classes/algolia/class-algolia.php |
Deleted (moved to new location) |
inc/classes/algolia/class-algolia-search.php |
Updates import to use new Algolia namespace |
inc/classes/algolia/class-algolia-index.php |
Updates import to use new Algolia namespace |
inc/classes/algolia/class-algolia-index-by-post.php |
Updates import to use new Algolia namespace |
uninstall.php |
Updates namespace reference for Algolia class |
inc/Main.php |
Registers new Search_Controller |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
I was also working on this file cleanup 😄 will rebase to your current branch.
up1512001
left a comment
There was a problem hiding this comment.
@justlevine Added few nits overall looks good as its just stripping basic options & admin_key removal.
|
Merged in #81 , then updated the new TSX endpoints to use
(As refactoring progresses, it may make sense to clobber different plugin-specific endpoints to a single transport for all governing side data in a single request ). More important than any of the above, is short-term shipability and long-term reshare/maintenance |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
up1512001
left a comment
There was a problem hiding this comment.
Approving just we need to fix initial site adding logic.
|
|
||
| setSites( data.shared_sites ); | ||
|
|
||
| if ( data.shared_sites.length === 0 ) { |
There was a problem hiding this comment.
this will not cause reload when I add first site which it should happen.
There was a problem hiding this comment.
This is the logic used by OneDesign and OneLogs, so I'm considering this nonblocking.
Description
This PR refactors and relocates the
Algoliaclient wrapper intoModules/Search. Other algolia-related files remain the same.Technical Details
algolia-credentialsendpoint was moved intoRest/Search_Controller(We're intentionally not getting it fromv2/settingsbecause we want only our endpoint to be allowed to decode it.)Governing_Datacall to that endpoint was moved into the new Algolia class as a private method.admin_keyfrom all areas of the backend (Yes I know I'll have to dedupe the conflicts once we merge chore: convert settings into TS & fix encryption issue #81The other classes were intentionally left out of this PR. I'm trying to keep the diffs small for fast code review before the weekend 🤞
Checklist
Screenshots
To-do
Fixes/Covers issue
Fixes #