Skip to content

feat: add changeBarcode to ProductOpenerApiV2#782

Open
garvit-bhattt wants to merge 9 commits intoopenfoodfacts:developfrom
garvit-bhattt:feat/add-changebarcode-api
Open

feat: add changeBarcode to ProductOpenerApiV2#782
garvit-bhattt wants to merge 9 commits intoopenfoodfacts:developfrom
garvit-bhattt:feat/add-changebarcode-api

Conversation

@garvit-bhattt
Copy link

What

  • Added the changeBarcode(currentCode, newCode, credentials) method manually to the ProductOpenerApiV2 class in src/off-v2.ts.
  • Bypassed the OpenAPI validation for new_code by casting the body to any, ensuring absolutely zero interference with other auto-generated schema files.

@VaiTon
Copy link
Member

VaiTon commented Feb 23, 2026

Bypassed the OpenAPI validation for new_code by casting the body to any, ensuring absolutely zero interference with other auto-generated schema files.

We should fix this at the OpenAPI layer if possible. Casting to any is never good.

…to any

This addresses the maintainer's feedback about removing the 'as any' cast from the changeBarcode request payload by using the updated OpenAPI schema.
@garvit-bhattt
Copy link
Author

@VaiTon I've regenerated the local OpenAPI types pointing to the updated openfoodfacts-server spec which now includes new_code, and I have removed the as any cast from the changeBarcode payload. Let me know if everything looks good now.

Copy link
Member

@VaiTon VaiTon left a comment

Choose a reason for hiding this comment

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

❯ head -n5 src/schemas/server/v2.ts
/**
 * This file was auto-generated by openapi-typescript.
 * Do not make direct changes to the file.
 */

v2.ts is regenerated using the OpenAPI definition stored in the server repository. See redocly.yaml and package.json for details.

@github-project-automation github-project-automation bot moved this from In progress to Review in progress in JavaScript SDK Feb 23, 2026
@garvit-bhattt
Copy link
Author

@VaiTon I've reverted the v2.ts changes. Now it just removes the as any cast in off-v2.ts.

@garvit-bhattt garvit-bhattt requested a review from VaiTon February 23, 2026 19:03
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.92%. Comparing base (f49e749) to head (3a6398d).
⚠️ Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
src/off-v2.ts 0.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #782      +/-   ##
===========================================
- Coverage    74.72%   73.92%   -0.81%     
===========================================
  Files           13       13              
  Lines          368      372       +4     
  Branches        82       84       +2     
===========================================
  Hits           275      275              
- Misses          69       73       +4     
  Partials        24       24              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@garvit-bhattt
Copy link
Author

@VaiTon Should I also add lightweight SDK-side checks (prevent same-barcode updates and basic barcode format validation) for better DX, or do you prefer relying fully on backend validation?

src/off-v2.ts Outdated
Comment on lines 341 to 342
user_id: credentials?.username ?? "",
password: credentials?.password ?? "",
Copy link
Member

@VaiTon VaiTon Feb 26, 2026

Choose a reason for hiding this comment

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

Can't we just use credentials?.username? Are these fields mandatory?

Copy link
Author

Choose a reason for hiding this comment

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

These fields are not strictly guaranteed to be present on credentials, so the fallback prevents undefined from being sent in the payload. Happy to simplify it.

Copy link
Member

Choose a reason for hiding this comment

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

I'd like to not even include the fields if the corresponding fields are undefined.

Copy link
Author

Choose a reason for hiding this comment

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

I'll update the payload to omit these fields when the corresponding credentials values are undefined.

Copy link
Author

Choose a reason for hiding this comment

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

@VaiTon I have pushed the changed requested

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Review in progress
Status: Backlog

Development

Successfully merging this pull request may close these issues.

3 participants