Skip to content

fix: link without identifier#5765

Merged
soyuka merged 3 commits into
api-platform:3.1from
soyuka:danny-issue-7522
Aug 22, 2023
Merged

fix: link without identifier#5765
soyuka merged 3 commits into
api-platform:3.1from
soyuka:danny-issue-7522

Conversation

@soyuka

@soyuka soyuka commented Aug 21, 2023

Copy link
Copy Markdown
Member
Q A
Branch? 3.1
Tickets Closes #5759 probably a bug report on slack by @dannyvw as well
License MIT
Doc PR na

@remi-vasco does this work?

@soyuka soyuka mentioned this pull request Aug 21, 2023
Closed
@soyuka soyuka requested a review from vincentchalamon August 21, 2023 17:26
vincentchalamon
vincentchalamon approved these changes Aug 22, 2023
View reviewed changes

@vincentchalamon vincentchalamon left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor change, then LGTM once tests are fixed.
Does this mean we can use the following syntax?

uriVariables: ['id']
// instead of
uriVariables: ['id' => new Link(fromClass: MyClass::class)]

Comment thread src/Metadata/Resource/Factory/NotExposedOperationResourceMetadataCollectionFactory.php Outdated
@soyuka

soyuka commented Aug 22, 2023

Copy link
Copy Markdown
Member Author

@vincentchalamon yes uriVariables: ['id'] is computed to uriVariables: ['id' => new Link(parameterName: 'id', identifiers: ['id'])]

@soyuka @vincentchalamon
use named arguments
1d555ec 
Co-authored-by: Vincent <407859+vincentchalamon@users.noreply.github.com>
@vincentchalamon

vincentchalamon commented Aug 22, 2023

Copy link
Copy Markdown
Contributor

@vincentchalamon yes uriVariables: ['id'] is computed to uriVariables: ['id' => new Link(parameterName: 'id', identifiers: ['id'])]

A PR to the doc would be great!

@soyuka

soyuka commented Aug 22, 2023

Copy link
Copy Markdown
Member Author

A PR to the doc would be great!

I'd prefer that people declare the whole Link as it may lead to unwanted behaviors with these "magic" shortcuts... It's mostly there for the backward compatibility with our previous identifiers...

@soyuka soyuka merged commit 07c9989 into api-platform:3.1 Aug 22, 2023
@soyuka soyuka deleted the danny-issue-7522 branch August 22, 2023 08:06
Romaixn pushed a commit to Romaixn/core that referenced this pull request Oct 3, 2023
@soyuka @vincentchalamon @Romaixn
fix(metadata): notexposed no urivariables inheritance (api-platform#5765
db331c2 
)

* fix(metadata): notexposed no urivariables inheritance

* use named arguments

Co-authored-by: Vincent <407859+vincentchalamon@users.noreply.github.com>

* fix tests

---------

Co-authored-by: Vincent <407859+vincentchalamon@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vincentchalamon vincentchalamon vincentchalamon approved these changes

Assignees

No one assigned

Labels

[RFR] Ready for Review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@soyuka @vincentchalamon