Move and refactor ai generator integrations#22313
Move and refactor ai generator integrations#22313pls78 wants to merge 24 commits intomove-and-refactor-ai-generator-rest-endpointsfrom
Conversation
Changed the get by role to presentation because the role is img only if alt has content, otherwise its presentation
as done in the rest of the places that import is done
rename directory
Co-authored-by: Igor <35524806+igorschoester@users.noreply.github.com>
pls78
added
the
changelog: non-user-facing
pls78
changed the base branch from
feature/ai-generator-in-free
to
move-and-refactor-ai-generator-rest-endpoints
…ode-to-free' into move-and-refactor-ai-generator-integrations
diedexx
left a comment
diedexx
left a comment
There was a problem hiding this comment.
Nice work!
Only some non-blocking comments
| */ | ||
| public function get_script_data(): array { | ||
| return [ | ||
| 'hasConsent' => $this->user_helper->get_meta( $this->user_helper->get_current_user_id(), '_yoast_wpseo_ai_consent', true ), |
There was a problem hiding this comment.
nit: It'd be nice if this was delegated to /application. Ideally the UI layer doesn't know about where things are stored.
The way I see it:
- ui/ConsentIntegration
- private app/ConsentQueryHandler
- app/ConsentQueryHandler
- private app/ports/ConsentRepositoryInterface
- app/ports/ConsentRepositoryInterface
- public get()
- public set()
- public delete()
- infra/(UserMeta)ConsentRepository implements ConsentRepositoryInterface
- private UserHelper
There was a problem hiding this comment.
I think it makes sense to extract the UserMeta out of the solution since that is very WordPress. And relying on the Application layer to get a repository that in this case happens to be a UserMeta repo since that is what WP uses 👍
There was a problem hiding this comment.
Not sure if we want to use the QueryHandler naming and things since we don't really do that a lot in the plugin. But this might be a nice time to start? 🤔 Might be good to do a little refresh on CQRS to know when to do a Command and when to do a Query
There was a problem hiding this comment.
Ah that doesn't matter too much. This is how we'd do it in the Platform team, but let's keep things consistent in each project 😄 As long as there's a way to get consent information from the app layer I'm more than happy
| * @return void | ||
| */ | ||
| public function register_hooks() { | ||
| if ( ! $this->options_helper->get( 'enable_ai_generator', false ) ) { |
There was a problem hiding this comment.
Isn't this covered by the AI_Conditional?
There was a problem hiding this comment.
Yes! I forgot to remove this test! 😅
Co-authored-by: Diede Exterkate <5352634+diedexx@users.noreply.github.com>
|
A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch. |
|
Close in favor of #22315 |
5 participants
Context
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
Relevant test scenarios
Test instructions for QA when the code is in the RC
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
UI changes
Other environments
[shopify-seo], added test instructions for Shopify and attached theShopifylabel to this PR.Documentation
Quality assurance
Innovation
innovationlabel.Fixes #556