-
Notifications
You must be signed in to change notification settings - Fork 826
AI Assistant: Add FeatureControl to Write Brief #39168
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
Conversation
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Follow this PR Review Process:
Still unsure? Reach out in #jetpack-developers for guidance! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
8e894d7
to
c7a50fe
Compare
@@ -88,6 +88,14 @@ export type TierValueProp = | |||
| Tier750Props[ 'value' ] | |||
| Tier1000Props[ 'value' ]; | |||
|
|||
export type FeatureControl = { | |||
enabled: boolean; | |||
'min-jetpack-version': string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I dreaming? 💯
) ) } | ||
{ features.map( | ||
feature => | ||
canWriteBriefFeatureBeEnabled( feature.config.name ) && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sub-feature control. Nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good and is working as expected.
The API response looks great, very well done on adding the default values for older features, as well as using nested features.
When the global "Write Brief" feature is disabled, the section on the UI is gone, as expected:
API Response | UI |
---|---|
![]() |
![]() |
When some sub-feature of "Write Brief" is disabled, the respective options are gone on the UI, as expected:
API Response | UI |
---|---|
![]() |
![]() |
An aside: after disabling some sub-features on the backend, I went to the editor and could see them visible for half a second, while the feature data was loaded. This is expected, as you discussed earlier today, and not a problem. We can make it better with loading states in the future, for example.
* add features control to ai-assistant-feature endpoint * check for write brief feature control * fix FeatureControl type * apply feature control checks * changelog * fix type * change default to enabled
Fixes #39167
Proposed changes:
Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions:
write-brief
or any of its subfeatures in D160005-codeOnly Write Brief checks for the function return value, other features will work regardless of their value.