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(jsonld): allow @id, @context and @type on denormalization 2 #6451

Merged
merged 4 commits into from
Jul 9, 2024

Conversation

ili101
Copy link
Contributor

@ili101 ili101 commented Jul 4, 2024

Q A
Branch? 3.3
Tickets Closes #6225
License MIT
Doc PR Feature already documented

Allaw @id, @type, @context with JsonLd. With this it's possible to use the @id as described in the documentation while using allow_extra_attributes: false.
More info in #6402.

@soyuka
Copy link
Member

soyuka commented Jul 4, 2024

this is perfect, would you be able to add a test?

features/main/standard_put.feature Outdated Show resolved Hide resolved
"bar": "b"
}
"""

Copy link
Member

Choose a reason for hiding this comment

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

Could you also add a test that ensures an error is thrown if the value of the @id key doesn't match the URL?

src/JsonLd/Serializer/ItemNormalizer.php Outdated Show resolved Hide resolved
@ili101
Copy link
Contributor Author

ili101 commented Jul 9, 2024

this is perfect, would you be able to add a test?

I was not available but I see a test was already added.
If it's useful in any way here is a PHPUnit test (commit) of the full usecase of writableLink using @id while allow_extra_attributes: false.

Side note I see you were discussing:

Could you also add a test that ensures an error is thrown if the value of the @id key doesn't match the URL?

If the @id line on the "Main" object is removed or id is different from the URL, the "subs" array embedded objects will behave unexpectedly (removed or throw a duplicate error). Not sure if a problem or just needs to be documented as the description in the documentation only mentions the need/behavior for the @id on the sub embedded objects and not their need on their parent too.

@@ -47,6 +49,29 @@ final class ItemNormalizer extends AbstractItemNormalizer
use JsonLdContextTrait;

public const FORMAT = 'jsonld';
public const JSONLD_KEYWORDS = [
Copy link
Member

Choose a reason for hiding this comment

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

This could maybe be an @internal enum?

Copy link
Member

Choose a reason for hiding this comment

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

too much trouble transforming an enum to an array this is more efficient imo, I could set private visibility?

@soyuka soyuka merged commit 32ef3d4 into api-platform:3.3 Jul 9, 2024
75 of 77 checks passed
@soyuka
Copy link
Member

soyuka commented Jul 9, 2024

thanks @ili101 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants