Repository navigation
feat: import recipe from schema.org JSON - #3430
TheTisiboth wants to merge 17 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. ℹ️ Recent review info
Walkthrough
Priority: Merge Risk:
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 56.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 37 functions across 14 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Issue [ #1872] requires a web UI that accepts pasted recipe JSON.AppNaviopensImportJsonDialog, and the dialog submits JSON throughrecipes.importJson. The/importcontroller path, `RecipeSer…Out of Scope Changes check The changed files support [ #1872]. Navigation and dialog files implement the UI. API handling, backend parsing, URL fallback, API documentation, and tests support direct JSON recipe import. No unrelat…Title check The title clearly and concisely describes the primary change: importing recipes from schema.org JSON. Description check The description covers the scope, user interface, API behavior, fallback URL handling, concerns, screenshots, tests, API documentation, and formal requirements. It is complete and aligned with the cha… Full details: Linked Issues check
-
Explanation
Issue [
#1872] requires a web UI that accepts pasted recipe JSON.AppNaviopensImportJsonDialog, and the dialog submits JSON throughrecipes.importJson. The/importcontroller path,RecipeService::importRecipeJson, andHttpJsonLdParser::parseJsonLdimplement the import. Tests cover controller handling, JSON-LD parsing, script-tag input, context handling, and URL fallback.
Full details: Out of Scope Changes check
-
Explanation
The changed files support [
#1872]. Navigation and dialog files implement the UI. API handling, backend parsing, URL fallback, API documentation, and tests support direct JSON recipe import. No unrelated change is demonstrated by the available evidence.
Full details: Description check
-
Explanation
The description covers the scope, user interface, API behavior, fallback URL handling, concerns, screenshots, tests, API documentation, and formal requirements. It is complete and aligned with the changes.
-
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
5b64a1c to
516f64f
Compare
Test Results 12 files 612 suites 2m 40s ⏱️ Results for commit 8defae9. ♻️ This comment has been updated with latest results. |
|
The branch comes from a forked repo, this is maybe why the label job is failing? (branch is read only) |
|
The failing check is known: nextcloud/.github#767 |
|
I just checked out the code and gave it a spin. Unfortunately, my first try to import a recipe failed. I took randomly a recipe from a website, extracted the JSON and fed that into the frame. For convinience, I copy the content into the following verbatim text: [{"@context":"https://schema.org","@graph":[{"@id":"https://www.chefkoch.de/rezepte/642141165243156/Brokkolisuppe.html#recipe","@type":"Recipe","aggregateRating":{"@type":"AggregateRating","bestRating":5,"ratingCount":732,"ratingValue":4.49,"worstRating":0},"alternativeHeadline":"Fett- und kalorienarm, schnell und sehr einfach","author":{"@id":"https://www.chefkoch.de/user/profil/98d8cb175968bdd9e846baa986d4b6da#author"},"comment":[{"@type":"Comment","author":{"@id":"https://www.chefkoch.de/user/profil/83d0d60bf611b8a2a19e14d89f1b2a68#author","@type":"Person","name":"AR53"},"datePublished":"2026-10-09","text":"Korrektur, sorry!\nEs sollte natürlich heissen „Mascarpone“ und nicht Marscapone.🤨"},{"@type":"Comment","author":{"@id":"https://www.chefkoch.de/user/profil/b9cbe56bef293b8ae6a03e46c456d2a5#author","@type":"Person","name":"Möppie"},"datePublished":"2016-07-08","text":"Lecker, habe noch etwas Muskatnuss und Zitrone dran gemacht sowie eine Prise Zucker. Meine Kinder (Brokkoli- und Kartoffelverächter) lieben sie - ich verkaufe sie mit einem fiesen Lachen als Hexensuppe mit Drachenschleim ;-) Liebe Grüsse ","upvoteCount":25},{"@type":"Comment","author":{"@id":"https://www.chefkoch.de/user/profil/d1c961196d3ba63c5d0921e34d280f9a#author","@type":"Person","name":"Ich2312"},"datePublished":"2025-01-03","text":"Mache Sie auch so nur eben auch mit Zwiebel und Knoblauch. Gemüsebrühe Salz Pfeffer Muskat etwas Curry ne butterflocke und n Schuss Sahne...Jammi","upvoteCount":11}],"commentCount":316,"cookTime":"PT30M","dateModified":"2026-10-09","datePublished":"2006-12-04","description":"Brokkolisuppe - fett- und kalorienarm, schnell und sehr einfach. Über 732 Bewertungen und für mega befunden. Mit ► Portionsrechner ► Kochbuch ► Video-Tipps!","image":{"@id":"https://www.chefkoch.de/rezepte/642141165243156/Brokkolisuppe.html#primaryimage"},"inLanguage":"de-DE","interactionStatistic":[{"@type":"InteractionCounter","interactionType":{"@type":"CommentAction"},"userInteractionCount":316}],"keywords":"Gemüse, Vorspeise, Suppe, warm, gebunden, einfach, kalorienarm, fettarm, gekocht","mainEntityOfPage":{"@id":"https://www.chefkoch.de/rezepte/642141165243156/Brokkolisuppe.html"},"name":"Brokkolisuppe von jesusfreak","nutrition":{"@type":"NutritionInformation","calories":"141 kcal","carbohydrateContent":"17 g","fatContent":"4.5 g","proteinContent":"7.7 g","servingSize":"1 Portion"},"prepTime":"PT15M","publisher":{"@id":"https://www.chefkoch.de/#organization"},"recipeCategory":"Vegetarisch","recipeCuisine":"Deutsch","recipeIngredient":["500 g Brokkoli","2 große Kartoffel(n)","1 Liter Gemüsebrühe","Salz und Pfeffer"],"recipeInstructions":[{"@type":"HowToSection","itemListElement":[{"@type":"HowToStep","position":1,"text":"Die Kartoffeln in kleine Stücke schneiden. ","url":"https://www.chefkoch.de/rezepte/642141165243156/Brokkolisuppe.html#step1"},{"@type":"HowToStep","position":2,"text":"Den Brokkoli und die Kartoffelstücke in der Brühe etwa 20 Minuten köcheln lassen, bis beides weich ist. Mit dem Mixstab pürieren oder alles in den Mixer geben. Wieder in den Topf zurückgeben und nochmals erwärmen. Dann mit Salz und Pfeffer abschmecken. Tipp: Zum Verfeinern evtl. noch 100 ml Sahne oder Pflanzensahne unterrühren, dann ist die Suppe allerdings auch etwas fettreicher.","url":"https://www.chefkoch.de/rezepte/642141165243156/Brokkolisuppe.html#step2"}],"name":"Zubereitung"}],"recipeYield":["4","4 Portionen"],"suitableForDiet":"https://schema.org/VegetarianDiet","totalTime":"PT45M","video":[{"@id":"https://www.chefkoch.de/rezepte/642141165243156/Brokkolisuppe.html#video"}]},{"@id":"https://www.chefkoch.de/rezepte/642141165243156/Brokkolisuppe.html#video","@type":"VideoObject","contentUrl":"https://img.chefkoch-cdn.de/ck.de/videos/4272-video.mp4","description":"Video zu Brokkolisuppe - fett- und kalorienarm, schnell und sehr einfach","duration":"PT33S","inLanguage":"de-DE","name":"Schnelle Brokkolisuppe","thumbnailUrl":"https://img.chefkoch-cdn.de/rezepte/642141165243156/bilder/1334973/crop-960x540/brokkolisuppe.jpg","uploadDate":"2020-09-10T00:00:00.000Z"},{"@id":"https://www.chefkoch.de/rezepte/642141165243156/Brokkolisuppe.html","@type":"WebPage","author":{"@id":"https://www.chefkoch.de/user/profil/98d8cb175968bdd9e846baa986d4b6da#author"},"breadcrumb":{"@id":"https://www.chefkoch.de/rezepte/642141165243156/Brokkolisuppe.html#breadcrumb"},"dateModified":"2026-10-09","datePublished":"2006-12-04","description":"Brokkolisuppe - fett- und kalorienarm, schnell und sehr einfach. Über 732 Bewertungen und für mega befunden. Mit ► Portionsrechner ► Kochbuch ► Video-Tipps!","inLanguage":"de-DE","isPartOf":{"@id":"https://www.chefkoch.de/#website"},"mainEntity":{"@id":"https://www.chefkoch.de/rezepte/642141165243156/Brokkolisuppe.html#recipe"},"name":"Brokkolisuppe von jesusfreak","primaryImageOfPage":{"@id":"https://www.chefkoch.de/rezepte/642141165243156/Brokkolisuppe.html#primaryimage"},"publisher":{"@id":"https://www.chefkoch.de/#organization"},"url":"https://www.chefkoch.de/rezepte/642141165243156/Brokkolisuppe.html"},{"@id":"https://www.chefkoch.de/rezepte/642141165243156/Brokkolisuppe.html#primaryimage","@type":"ImageObject","contentUrl":"https://img.chefkoch-cdn.de/rezepte/642141165243156/bilder/1334973/crop-960x540/brokkolisuppe.jpg","height":540,"url":"https://img.chefkoch-cdn.de/rezepte/642141165243156/bilder/1334973/crop-960x540/brokkolisuppe.jpg","width":960},{"@id":"https://www.chefkoch.de/rezepte/642141165243156/Brokkolisuppe.html#breadcrumb","@type":"BreadcrumbList","itemListElement":[{"@type":"ListItem","item":"https://www.chefkoch.de/","name":"Startseite","position":1},{"@type":"ListItem","item":"https://www.chefkoch.de/rezepte/","name":"Rezepte","position":2},{"@type":"ListItem","item":"https://www.chefkoch.de/rs/s0g32/Speisearten.html","name":"Spezielles","position":3},{"@type":"ListItem","item":"https://www.chefkoch.de/rs/s0t6082/Ernaehrungskonzepte-Rezepte.html","name":"Ernährungskonzepte","position":4},{"@type":"ListItem","item":"https://www.chefkoch.de/rs/s0t32/Vegetarisch-Rezepte.html","name":"Vegetarisch","position":5}]},{"@id":"https://www.chefkoch.de/#website","@type":"WebSite","inLanguage":"de-DE","name":"Chefkoch","publisher":{"@id":"https://www.chefkoch.de/#organization"},"url":"https://www.chefkoch.de/"},{"@id":"https://www.chefkoch.de/#organization","@type":"Organization","logo":{"@type":"ImageObject","height":144,"url":"https://img.chefkoch-cdn.de/img/ck.de/ck-logo-de.png","width":144},"name":"Chefkoch","sameAs":["https://www.facebook.com/chefkoch.de","https://www.instagram.com/chefkoch/","https://www.youtube.com/chefkoch","https://de.pinterest.com/chefkochde/","https://www.tiktok.com/@chefkoch","https://www.whatsapp.com/channel/0029VabvqZU3LdQOknGWyl1H"],"url":"https://www.chefkoch.de/"},{"@id":"https://www.chefkoch.de/user/profil/98d8cb175968bdd9e846baa986d4b6da#author","@type":"Person","name":"jesusfreak","url":"https://www.chefkoch.de/user/profil/98d8cb175968bdd9e846baa986d4b6da"}]}]I think the problem is the rather minimalistic Schema.org parser/validator. However, it might be a good idea in my mind to keep the parser in one location, backend or frontend. We will need it in the backend anyways, so if we filter in the frontend as well, this might rule out some content/JSON structures that would otherwise work. Is there a reason for not directly storing it in the backend (just asking, maybe I overlooked something)? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/Unit/Service/RecipeExtractionServiceTest.php (1)
89-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix the PHPDoc
@vartag to remove the PHPStan error.
createMock()returnsMockObject&HttpJsonLdParser. PHPStan reportsvarTag.nativeTypefor the union tag. Use an intersection type.Proposed fix
- /** @var HttpJsonLdParser|MockObject $jsonParser */ + /** @var HttpJsonLdParser&MockObject $jsonParser */Source: Linters/SAST tools
lib/Service/RecipeService.php (1)
377-385: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd the missing
checkRecipestep and a docblock.
downloadRecipecallscheckRecipebeforeaddRecipe.addRecipealso callscheckRecipe, so the behavior is safe. The method still lacks a docblock. Add one that lists theImportExceptionand the recipe-exists exception.lib/Controller/Implementation/RecipeImplementation.php (1)
324-340: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRequest with both
urlandjsonsilently ignoresjson.When a client sends both fields, the code takes the URL path. This is acceptable. Document the precedence in the API spec and changelog.
A non-string
urlstill reachesisDownloadUrlValid(string $url). That behavior existed before this change.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
99f929e6-984c-4937-9007-c1b5fd4c8ed7
📒 Files selected for processing (12)
docs/dev/api/changelog/0.mdlib/Controller/Implementation/RecipeImplementation.phplib/Helper/HTMLParser/HttpJsonLdParser.phplib/Service/JsonService.phplib/Service/RecipeExtractionService.phplib/Service/RecipeService.phpsrc/components/Modals/ImportJsonDialog.vuesrc/js/api-interface.jstests/Unit/Controller/Implementation/RecipeImplementationTest.phptests/Unit/Helper/HTMLParser/HttpJsonLdParserTest.phptests/Unit/Service/JsonServiceTest.phptests/Unit/Service/RecipeExtractionServiceTest.php
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Preserve the schema context when searching a nested graph. · HttpJsonLdParser.php:191
lib/Helper/HTMLParser/HttpJsonLdParser.php:191
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the schema context when searching a nested graph.
If an outer graph supplies
@contextand an inner graph omits it,mapGraphField($item)finds the inner Recipe without copying the outer context.searchForRecipeInArraythen checks that Recipe with$haveSchemaContext = true, butparseJsonLdlater requires the Recipe to contain its own context. The import fails with “No recipe was found.” Pass the inherited context into the nested search or copy it onto the selected Recipe.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
d8be6b51-ad88-46ce-8dfe-f89d093a237c
📒 Files selected for processing (3)
lib/Helper/HTMLParser/HttpJsonLdParser.phplib/Service/RecipeService.phptests/Unit/Helper/HTMLParser/HttpJsonLdParserTest.php
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
|
Thanks for testing. |
509c88d to
1ab0ff6
Compare
Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Léo Jan <thetisiboth@gmail.com>
Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Léo Jan <thetisiboth@gmail.com>
Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Léo Jan <thetisiboth@gmail.com>
Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Léo Jan <thetisiboth@gmail.com>
Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Léo Jan <thetisiboth@gmail.com>
Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Léo Jan <thetisiboth@gmail.com>
1ab0ff6 to
207898a
Compare
…recipe Signed-off-by: Léo Jan <thetisiboth@gmail.com>
Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Léo Jan <thetisiboth@gmail.com>
Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Léo Jan <thetisiboth@gmail.com>
|
open api specs has been updated, and coderabbittai review is handled |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
b10edbfe-74fb-489c-847b-85d5594bfb68
📒 Files selected for processing: 8
docs/dev/api/trunk/internal-cookbook.yamldocs/dev/api/trunk/objects.yamldocs/dev/api/trunk/openapi-cookbook.yamllib/Helper/Filter/JSON/FixUrlFilter.phplib/Helper/HTMLParser/HttpJsonLdParser.phplib/Service/RecipeService.phptests/Unit/Helper/Filter/JSON/FixUrlFilterTest.phptests/Unit/Helper/HTMLParser/HttpJsonLdParserTest.php
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Léo Jan <thetisiboth@gmail.com>
Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Léo Jan <thetisiboth@gmail.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Extract mainEntityOfPage.url before sanitizing it. · FixUrlFilter.php:40-51
lib/Helper/Filter/JSON/FixUrlFilter.php:40-51
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winExtract
mainEntityOfPage.urlbefore sanitizing it.A valid schema.org
WebPagecan containurlwithout@id. The current fallback assigns the complete WebPage object tourl.filter_var()returnsfalsefor that array, and the next validation replaces it with an empty string. The import then persists no source URL.Suggested fix
- $json[self::URL] = $json['mainEntityOfPage']['@id'] ?? $json['mainEntityOfPage']; + $json[self::URL] = $json['mainEntityOfPage']['@id'] ?? $json['mainEntityOfPage']['url'] ?? $json['mainEntityOfPage'];
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
c9db9b65-d858-4fe4-8dbb-a78b047004dd
📒 Files selected for processing: 4
docs/dev/api/trunk/internal-cookbook.yamldocs/dev/api/trunk/openapi-cookbook.yamllib/Helper/HTMLParser/HttpJsonLdParser.phptests/Unit/Helper/HTMLParser/HttpJsonLdParserTest.php
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Léo Jan <thetisiboth@gmail.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
ad2db5a to
fcfff25
Compare
Merge origin/master into feat/import-json-recipe. URL string validation and trimming from master are guarded behind isset($data['url']) to preserve JSON-only import path. Both new imports (HttpJsonLdParser, ImageConversionHelper) and constructor params are kept. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Léo Jan <leo.jan@doctolib.com>
|
This PR/issue depends on: |
…cipe Signed-off-by: Léo Jan <leo.jan@doctolib.com>
christianlupus
left a comment
christianlupus
left a comment
There was a problem hiding this comment.
I did not yet find the time to give it a complete review yet. However, I wanted to give one point, that will cause trouble, to be honest: the openAPI spec is not as required by the specs.
Assisted-by: Claude Code:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Léo Jan <leo.jan@doctolib.com>
Assisted-by: Claude Code:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Léo Jan <leo.jan@doctolib.com>
Topic and Scope
Closes #1872
Depends on #3507
Depends on #3454
Adds a way to create a recipe by pasting schema.org JSON.
<script>tagScreenshots
Concerns/issues
Formal requirements
There are some formal requirements that should be satisfied. Please mark those by checking the corresponding box.
AI assistance
I used AI to help me run the investigation, to check if this feature didn't exist yet, generate the code and tests