Skip to content

feature: allow plain identifiers in json api #1834

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

Conversation

Simperfit
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #1833
License MIT
Doc PR

This PRs allows to use "allow_plain_identifiers" in JSON API.

@soyuka
Copy link
Member

soyuka commented Apr 9, 2018

You should've asked first. This is joining my though in #1716 (comment)

Using ItemDataProvider in the normalizers (this one or the abstract one) is wrong. We should only use IriConverter. This is even worse with #1478 because the ItemDataProvider needs to be called with normalized identifiers...
Also, IriConverter is the class we want to be responsible for handling data in the normalizers.

@Simperfit this is the fix I want to propose, I don't have much time though so feel free to do it :).

  1. Add a method in IriConverter that has the following signature:
public function getIriFromPlainIdentifier($id, $resourceClass) {
 // returns an IRI
}

Or this alternative (not fond of this because IriConverter should only do things based on IRIs:

public function getItemFromPlainIdentifier($id, $resourceClass) {}
  1. Use that IRI in the normalization processes
  2. Rewrite a proper fix for throw NotFound exception when use plain identifiers #1716 related issue

@Simperfit
Copy link
Contributor Author

Simperfit commented Apr 9, 2018

Ow @soyuka I didn't know that was already a topic!

I can implement the first solution as it seem (to me too) to be the best one.

So I'm going to (in a new PR):

  • remove ItemDataProvider from the abstract class.
  • implement getIriFromPlainIdentifier and use it in the abstract class
  • and use this new getIriFromPlainIdentifier in the JSONAPI's ItemNormalizer.

@dunglas
Copy link
Member

dunglas commented May 2, 2018

You should create a new interface PlainIdentifierConverterInterface for that (it's not the responsibility of the IriConverterInterface).

@flug
Copy link
Contributor

flug commented Nov 14, 2019

@Simperfit Up is this pull request still valid?

@soyuka
Copy link
Member

soyuka commented Nov 15, 2019

Let's close this for now and keep track of it in #2686

@soyuka soyuka closed this Nov 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants