Skip to content
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

refactor(ContributionAssistant): Full integration with price tag data #1166

Merged

Conversation

TTalex
Copy link
Collaborator

@TTalex TTalex commented Dec 23, 2024

What

  • The contribution assistant now reads (as input), creates (after drawing) and updates price tags (after price creation)
  • When no new price tags are drawn, this significantly improves the speed of the contribution assistant for existing proofs, since it no longer requires waiting for gemini. If new price tags are drawn, we still have to wait for backend processing.
  • It also supports skipping the proof upload step when a query parameter contains proofs_ids to handle, e.g: /experiments/contribution-assistant?proofs=[1512,1513]
  • After uploading the prices, users can press the new "Next Proof" button to go back and handle the next proof_id in params.

Demo

Example, starting from a not yet implemented multiple proof upload CTA
https://github.com/user-attachments/assets/517d535f-8283-42f3-957d-94338af3d4c3

Another example, showing how the assistant can be used to add price tags manually (the price validator assistant is used to show an increasing number of price tags, before and after)
https://github.com/user-attachments/assets/14a20851-e269-44fc-b5dd-c6278c7ef726

Note that currently, it only supports adding new price tags, and does not handle removing them.

@@ -209,84 +209,133 @@ export default {
// Summary tab should be enabled when there are product prices to be added and the add prices process is either running or done
const enableSummaryTab = this.productPriceForms.length && (this.addPricesLoading || this.allDone)
return !enableSummaryTab
},
proofsIdsFromQueryParam() {
return JSON.parse(this.$router.currentRoute.value.query.proofs)
Copy link
Member

@raphodn raphodn Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we use proof_ids=id1,id2 instead of proofs=[id1,id2] ?

  • rename proofs to proof_ids
  • use comma-seperated values

why ? to be more html/API-compliant (well i'm not sure there's hard rules, but arrays in url feels wrong ^^)
example of comma-seperated usage https://search.openfoodfacts.org/docs#/default/search_get_search_get

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an alternative could also be proof_id=id1&next_proof_id=id2

Copy link
Collaborator Author

@TTalex TTalex Dec 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I spent a bit of time looking for a standardized way of passing arrays in url params. It's quite surprising that there are none. I went with this one for ease of use using JSON, but I do not like it either 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

proof_ids=id1,id2 has my preference, we already use it for product lists in ProductOpener

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed here: 07711e7

@raphodn raphodn force-pushed the ttalex/contribution-assistant-full-pricetag-integration branch from 580b837 to e4d8c41 Compare December 24, 2024 18:29
@raphodn raphodn merged commit 54a1035 into master Dec 24, 2024
6 of 7 checks passed
@raphodn raphodn deleted the ttalex/contribution-assistant-full-pricetag-integration branch December 24, 2024 23:19
raphodn added a commit that referenced this pull request Jan 1, 2025
raphodn added a commit that referenced this pull request Jan 3, 2025
raphodn added a commit that referenced this pull request Jan 3, 2025
raphodn added a commit that referenced this pull request Jan 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

AI-powered price addition workflow (Contribution assistant experiment)
3 participants