-
-
Notifications
You must be signed in to change notification settings - Fork 130
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
Add embedded support #299
Add embedded support #299
Conversation
7cbb6eb
to
a016316
Compare
@@ -72,7 +74,7 @@ export const transformJsonLdDocumentToReactAdminDocument = ( | |||
false, | |||
); | |||
} | |||
document[key] = document[key]['@id']; | |||
document[key] = useEmbedded ? document[key] : document[key]['@id']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
document[key] = useEmbedded ? document[key] : document[key]['@id']; | |
document[key.toString()] = useEmbedded ? document[key.toString()] : document[key.toString()]['@id']; |
To avoid Object Injection Sink IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It cannot be done like this because React Admin data providers need to give record objects: https://marmelab.com/react-admin/DataProviders.html#response-format
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't affect return, your key
var should be a string right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes, you're right sorry. But Object.keys
returns only strings so I don't think it's necessary.
Hey guys, thanks for your work! Unfortunately, those changes doesn't seem to work for me (or I am doing something totally wrong). I am using "useEmbedded=true" and my embedded objects are not resolved :( I also followed your docu in Further details you can finde here: I just debugged it in the browser a bit.... shouldn't the "useEmbedded" parameter be passed inside the recursive function calls of transformJsonLdDocumentToReactAdminDocument()? i can see
=> nested seem to be quite limited here! |
Alternative of #107.
Since
@api-platform/api-doc-parser
can now tell if a field is an embedded one, the admin can be slightly smarter and not using a reference for it.If it's an array of embeddeds, the previous behavior stays the same: objects are replaced by their IRI and references are used in
FieldGuesser
andInputGuesser
.However, if it's an embedded field, the behavior changes depending of the value of
useEmbedded
in the Hydra data provider:false
, the IRI of the embedded will be displayed as a text (BC: it was a reference before, you can get back this behaviour by replacing the guessers by references) inFieldGuesser
andInputGuesser
.true
, the embedded object will be displayed as a text inFieldGuesser
andInputGuesser
.The
useEmbedded
isfalse
by default to not cause too much breaking changes, but it will not exist anymore in 3.0 (and the behavior will be like it was alwaystrue
).It means that, if you enable
useEmbedded
in the Hydra data provider, you can now use the dot notation of React Admin for complex structures!However, the admin cannot determine the structure of the embedded and the fields in it. It's because the Hydra documentation doesn't give this information. In order to display them, you need to use directly the React Admin components.