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

fix: fields=all returns customized field values #7771

Merged
merged 5 commits into from
Nov 30, 2022
Merged

Conversation

stephanegigandet
Copy link
Contributor

fields=all had a difficult to understand behaviour: it returned all raw fields, without customization. Which meant that asking for packagings in v3 would return a different packagings field that asking for all in v3.

This PR:

  • adds a new fields=raw option, to get whatever is stored in MongoDB
  • makes values from fields=all customized
  • add the possibility to request all + some fields that are computed on requests (e.g. fields=all,attribute_groups or fields=updated,knowledge_panels
  • adds corresponding tests

@stephanegigandet stephanegigandet requested a review from a team as a code owner November 29, 2022 15:06
@teolemon teolemon added API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) API WRITE WRITE API to allow sending product info and image API READ All READ APIs include Product, Search… API v3 labels Nov 29, 2022
Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

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

LGTM. It's a good change.

I only have small suggestions

docs/reference/api-v3.yml Outdated Show resolved Hide resolved
@@ -58,7 +81,7 @@ paths:
description: Barcode of the product
patch:
summary: WRITE Product - Create or update product (API V3 - Implementation in progress)
operationId: post-api-v3-product-barcode
operationId: patch-api-v3-product-barcode
responses:
'200':
description: |-
Copy link
Member

Choose a reason for hiding this comment

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

Why is the explanation below not in the content schema with an entry "fields" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a fields entry in the content schema for the request (which is above), below is the content schema of the response (which does not have a fields entry, but is affected by the fields parameter in the query)

@@ -74,7 +74,7 @@
"2xxxxxxxxxxx"
],
"complete" : 0,
"completeness" : "0.5",
"completeness" : 0.5,
Copy link
Member

Choose a reason for hiding this comment

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

Do you understand why you have this flip-flap between string and raw numbers ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexgarel it's an issue with JSON:PP which select the type (string or number) based on what the last use of the scalar was. I tried to find out what exactly was using those scalars in a string context (like a debug dump function), but I failed.

https://docs.mojolicious.org/JSON/PP#PERL---JSON

Co-authored-by: Alex Garel <alex@garel.org>
@github-actions github-actions bot added 📚 Documentation Documentation issues improve the project for everyone. 🧪 tests and removed API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) labels Nov 30, 2022
@github-actions github-actions bot added the API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) label Nov 30, 2022
@sonarcloud
Copy link

sonarcloud bot commented Nov 30, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@stephanegigandet stephanegigandet merged commit aa7ac73 into main Nov 30, 2022
@stephanegigandet stephanegigandet deleted the fields-all branch November 30, 2022 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API READ All READ APIs include Product, Search… API v3 API WRITE WRITE API to allow sending product info and image API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) 📚 Documentation Documentation issues improve the project for everyone. 🧪 tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants