-
Notifications
You must be signed in to change notification settings - Fork 134
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
[Product AI v2] generation prompt + option selector #11993
Conversation
Generated by 🚫 Danger |
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
@@ -75,6 +75,67 @@ object AIPrompts { | |||
|
|||
@Suppress("LongParameterList") | |||
fun generateProductCreationPrompt( |
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.
Instead of putting lots of ifs
to check the FeatureFlag
inside the prompt, I decided to create different functions and make the changes there, and I added the suffix Legacy
for the previous versions to make it easier to spot them for cleanup.
existingCategories: List<ProductCategory>, | ||
existingTags: List<ProductTag> | ||
): AIProductModel { | ||
val jsonModel = JSONObject(json) |
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.
Instead of using Gson
for the parsing, I decided to do it manually, I found this easier for allowing me to match against the existing categories/tags and assign them directly.
private var isProductCategoriesFetched = false | ||
private var isProductTagsFetched = false |
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.
These properties will allow us to skip fetching categories and tags when the user uses the button Re-generate
...rce/src/main/kotlin/com/woocommerce/android/ui/products/ai/preview/AiProductPreviewScreen.kt
Outdated
Show resolved
Hide resolved
return Result.failure(it) | ||
} | ||
|
||
if (!::languageISOCode.isInitialized) { |
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.
Nice TIL about this way of checking if lateinit
property is initialized
Excellent job @hichamboushaba. Just left a minor nitpick. But the PR is ready for shipping once fluxC changeset is updated |
Co-authored-by: Jorge Mucientes <jorge.mucientes@automattic.com>
# Conflicts: # build.gradle
fc3604e
to
3241a72
Compare
Found 1 violations: The PR caused the following dependency changes:expand
-+--- org.wordpress:fluxc:trunk-56cd6652d0d83fccfbb7ecc19cf2c6496d812dab
-| +--- org.wordpress:wellsql:2.0.0
-| | +--- androidx.annotation:annotation:1.2.0 -> 1.7.0 (*)
-| | \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
-| +--- org.wordpress.fluxc:fluxc-annotations:trunk-56cd6652d0d83fccfbb7ecc19cf2c6496d812dab
-| +--- org.greenrobot:eventbus:3.3.1 (*)
-| +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
-| +--- com.android.volley:volley:1.1.1 -> 1.2.0
-| +--- androidx.paging:paging-runtime:2.1.2
-| | +--- androidx.paging:paging-common:2.1.2
-| | | +--- androidx.annotation:annotation:1.0.0 -> 1.7.0 (*)
-| | | \--- androidx.arch.core:core-common:2.0.0 -> 2.2.0 (*)
-| | +--- androidx.arch.core:core-runtime:2.0.0 -> 2.2.0 (*)
-| | +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.6.2 (*)
-| | +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.6.2 (*)
-| | \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
-| +--- com.goterl:lazysodium-android:5.0.2
-| +--- net.java.dev.jna:jna:5.5.0
-| +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.9.10 (*)
-| +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.20 -> 1.9.22
-| | \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.22 (*)
-| +--- androidx.appcompat:appcompat:1.0.2 -> 1.6.1 (*)
-| +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
-| +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6
-| | \--- androidx.annotation:annotation:1.2.0 -> 1.7.0 (*)
-| +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
-| | +--- androidx.annotation:annotation:1.1.0 -> 1.7.0 (*)
-| | +--- com.google.crypto.tink:tink-android:1.5.0
-| | \--- androidx.collection:collection:1.1.0 -> 1.4.0 (*)
-| +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
-| | +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
-| | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.9.10 (*)
-| +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-| +--- org.apache.commons:commons-text:1.10.0 (*)
-| +--- androidx.room:room-runtime:2.4.2 -> 2.5.2 (*)
-| +--- androidx.room:room-ktx:2.4.2 -> 2.5.2
-| | +--- androidx.room:room-common:2.5.2 (*)
-| | +--- androidx.room:room-runtime:2.5.2 (*)
-| | +--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.9.22 (*)
-| | \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4 -> 1.8.1 (*)
-| +--- com.google.dagger:dagger:2.42 -> 2.50
-| | \--- javax.inject:javax.inject:1
-| +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.8.1 (*)
-| \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.8.1 (*)
++--- org.wordpress:fluxc:trunk-025008c28331ecf1b4f69009348b01bad42bac99 FAILED
-+--- org.wordpress.fluxc.plugins:woocommerce:trunk-56cd6652d0d83fccfbb7ecc19cf2c6496d812dab
-| +--- org.wordpress:wellsql:2.0.0 (*)
-| +--- org.wordpress.fluxc:fluxc-annotations:trunk-56cd6652d0d83fccfbb7ecc19cf2c6496d812dab
-| +--- androidx.room:room-ktx:2.4.2 -> 2.5.2 (*)
-| +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.9.10 (*)
-| +--- org.wordpress:fluxc:trunk-56cd6652d0d83fccfbb7ecc19cf2c6496d812dab (*)
-| +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-| +--- com.google.dagger:dagger:2.42 -> 2.50 (*)
-| +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.8.1 (*)
-| +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.8.1 (*)
-| \--- androidx.room:room-runtime:2.4.2 -> 2.5.2 (*)
++--- org.wordpress.fluxc.plugins:woocommerce:trunk-025008c28331ecf1b4f69009348b01bad42bac99 FAILED
+--- org.wordpress:login:1.17.0
| +--- com.github.bumptech.glide:glide:4.12.0 -> 4.16.0
-| | \--- androidx.exifinterface:exifinterface:1.3.6 (*)
+| | \--- androidx.exifinterface:exifinterface:1.3.6
+| | \--- androidx.annotation:annotation:1.2.0 -> 1.7.0 (*)
-| \--- com.google.dagger:dagger:2.47 -> 2.50 (*)
+| \--- com.google.dagger:dagger:2.47 -> 2.50
+| \--- javax.inject:javax.inject:1
\--- project :libs:cardreader
+--- com.stripe:stripeterminal-localmobile:3.1.1
| \--- org.jetbrains.kotlin:kotlin-parcelize-runtime:1.8.22 -> 1.9.22
- | \--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.9.22 (*)
+ | \--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.9.22
+ | \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.22 (*)
\--- com.stripe:stripeterminal-core:3.1.1
- +--- androidx.security:security-crypto:1.1.0-alpha03 (*)
+ +--- androidx.security:security-crypto:1.1.0-alpha03
+ | +--- androidx.annotation:annotation:1.1.0 -> 1.7.0 (*)
+ | +--- com.google.crypto.tink:tink-android:1.5.0
+ | \--- androidx.collection:collection:1.1.0 -> 1.4.0 (*)
- \--- androidx.room:room-ktx:2.5.2 (*)
+ \--- androidx.room:room-ktx:2.5.2
+ +--- androidx.room:room-common:2.5.2 (*)
+ +--- androidx.room:room-runtime:2.5.2 (*)
+ +--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.9.22 (*)
+ \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4 -> 1.8.1 (*)
Please review and act accordingly
|
Closes: #11972 Closes: #11973
Please don't merge until the FluxC hash is updated.
Description
This PR adds the following changes:
The generation fails sometimes, this is because we still need to implement this part #11929, if you find issues with testing the PR, please let me know and I can suggest a temporary solution.This was updated, the PR includes the changes of the PR wordpress-mobile/WordPress-FluxC-Android#3049 which implements the required change
Testing information
Images/gif
Note: the buttons look bigger than what we have in Figma, that's because Compose enforces 48dp as the minimum size for touch targets, I believe we can override the minimum, but I don't think we should do it, the accessibility is more important than having it match Figma perfectly, WDYT?
RELEASE-NOTES.txt
if necessary. Use the "[Internal]" label for non-user-facing changes.