Skip to content

Update Hydra JsonSchema context property possible types#4223

Merged
alanpoulain merged 4 commits into
api-platform:2.6from
llupa:2.5
Apr 27, 2021
Merged

Update Hydra JsonSchema context property possible types#4223
alanpoulain merged 4 commits into
api-platform:2.6from
llupa:2.5

Conversation

@llupa

@llupa llupa commented Apr 15, 2021

Copy link
Copy Markdown
Contributor
Q A
Branch? 2.6
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #4222
License MIT

JsonSchema assertion will always fail for API Resources that have an output class defined.

@llupa

llupa commented Apr 15, 2021

Copy link
Copy Markdown
Contributor Author

I got lost in the test classes! Can someone show me where is the best place to add a test for this?

Comment thread src/Hydra/JsonSchema/SchemaFactory.php Outdated
@alanpoulain

Copy link
Copy Markdown
Member

SchemaFactoryTest is a good start for the tests.

@llupa llupa marked this pull request as draft April 16, 2021 12:17
@llupa

llupa commented Apr 16, 2021

Copy link
Copy Markdown
Contributor Author

@alanpoulain Hello. I updated the PR. I saw that this: ContextBuilderTest is checking that the body of an anon. class normalization has an 'object' @context.

I was thinking to add an integration test where I assert the schema for both a resource class and an anon. class, but I am not sure how to use the fake Models in tests 🤔

@llupa llupa marked this pull request as ready for review April 16, 2021 16:49
@judmir

judmir commented Apr 23, 2021

Copy link
Copy Markdown

We have the exact same issue at our project. Would be really great if this PR can be merged.

@alanpoulain

Copy link
Copy Markdown
Member

I've added an integration test in ApiTestCaseTest (I think it's the best place to do so).

@alanpoulain alanpoulain merged commit 13112d3 into api-platform:2.6 Apr 27, 2021
@alanpoulain

Copy link
Copy Markdown
Member

Thank you @llupa.

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