Open Food Facts as search provider#1294
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds a comprehensive integration test suite for the ItemCategories REST CRUD endpoints alongside supporting test infrastructure configuration. The test class validates successful operations, error handling, and field preservation behavior. Configuration updates include gitignore patterns for development tooling and mock service mappings for external API error scenarios. ChangesItemCategories CRUD Integration Tests and Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/interfaces/endpoints/inventory/ItemCategoriesCrudTest.java (1)
150-150: 💤 Low valueAvast! Use the imported ObjectId rather than its full title.
Lines 150 and 165 be using the full
org.bson.types.ObjectId()whenObjectIdalready be imported at line 9. Redundant it is—keep it shipshape by using justObjectId()like a proper deckhand.⚓ Proposed trim for the rigging
- ErrorMessage errorMessage = setupJwtCall(given(), testUser.getAttributes().get(TestUserService.TEST_JWT_ATT_KEY)) - .pathParam("oqmDbIdOrName", DEFAULT_TEST_DB_NAME) - .pathParam("id", new org.bson.types.ObjectId().toHexString()) + ErrorMessage errorMessage = setupJwtCall(given(), testUser.getAttributes().get(TestUserService.TEST_JWT_ATT_KEY)) + .pathParam("oqmDbIdOrName", DEFAULT_TEST_DB_NAME) + .pathParam("id", new ObjectId().toHexString())And line 165 similarly:
- ErrorMessage errorMessage = setupJwtCall(given(), testUser.getAttributes().get(TestUserService.TEST_JWT_ATT_KEY)) - .pathParam("oqmDbIdOrName", DEFAULT_TEST_DB_NAME) - .pathParam("id", new org.bson.types.ObjectId().toHexString()) + ErrorMessage errorMessage = setupJwtCall(given(), testUser.getAttributes().get(TestUserService.TEST_JWT_ATT_KEY)) + .pathParam("oqmDbIdOrName", DEFAULT_TEST_DB_NAME) + .pathParam("id", new ObjectId().toHexString())Also applies to: 165-165
software/plugins/external-item-search/.gitignore (1)
45-46: 💤 Low valueHoist the colors on partial redundancy in the gitignore sail.
Lines 45–46 add recursive patterns for
.settings/and.projectusing the**/prefix, but the root-level versions already exist at lines 6 and 8. The recursive patterns will catch those too, but the old patterns remain. While no functional harm be done, it be cluttering the manifest. Consider consolidating by removing the root-only patterns and keeping only the recursive ones—or keeping root-only if the intent be to match only at the project root.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ecb8b30c-e0ba-48f3-bda9-7e6e9444cc79
📒 Files selected for processing (7)
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/interfaces/endpoints/inventory/ItemCategoriesCrudTest.javasoftware/plugins/external-item-search/.gitignoresoftware/plugins/external-item-search/dev/services/mappings/openfoodfacts/README.mdsoftware/plugins/external-item-search/dev/services/mappings/openfoodfacts/openfoodfacts_barcode.jsonsoftware/plugins/external-item-search/dev/services/mappings/openfoodfacts/openfoodfacts_barcode_not_found.jsonsoftware/plugins/external-item-search/dev/services/mappings/openfoodfacts/openfoodfacts_search.jsonsoftware/plugins/external-item-search/dev/services/mappings/openfoodfacts/openfoodfacts_search_not_found.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CI-Pipeline / Build
- GitHub Check: CI-Pipeline / Build
🔇 Additional comments (1)
software/plugins/external-item-search/dev/services/mappings/openfoodfacts/openfoodfacts_search_not_found.json (1)
1-28: LGTM! This mock mapping be trimmed and ready—proper structure for an empty search result at HTTP 200. All fields present and accounted for. Shipshape!
| @@ -0,0 +1,40 @@ | |||
| { | |||
| "request": { | |||
| "urlPathPattern": "/openfoodfacts/api/v3/product/30469200297590000", | |||
There was a problem hiding this comment.
Blimey! The barcode in the response body don't match the request URL—a breach in the hull!
Mark me words: the request be seeking product barcode 30469200297590000 (17 digits), but the response body declares code 304692002975000 (15 digits—missing the leading "00"). This mismatch means the mock may not be matched correctly by the test infrastructure, or if matched, the client code checking the returned code against the requested barcode will spot a discrepancy.
Correct the code field in the response body to 30469200297590000 to match the barcode in the request URL.
⚓ Proposed fix to seal the breach
"jsonBody": {
- "code": "304692002975000",
+ "code": "30469200297590000",
"errors": [Also applies to: 12-12
axgiri
force-pushed
the
dev/698-plugin-ExtItemSearch-add-openfoodfacts-as-seatch-provider
branch
from
749d886 to
69a4aa3
Compare
axgiri
changed the title
|
will work on #1295 |
2 participants
Checklist:
#698
Summary by CodeRabbit