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

MetadataInterface::getSpace() return type incorrect #91

Open
ChrisDBrown opened this issue Jun 7, 2019 · 7 comments
Open

MetadataInterface::getSpace() return type incorrect #91

ChrisDBrown opened this issue Jun 7, 2019 · 7 comments

Comments

@ChrisDBrown
Copy link
Contributor

Trying to get a smaller replication case, but currently seeing a case where Markup/Contentful/DynamicEntry->getSpace() returns Markup/Contentful/Link which is not of type Markup/Contentful/SpaceInterface as expected.

@ChrisDBrown
Copy link
Contributor Author

@ChrisDBrown
Copy link
Contributor Author

@ChrisDBrown
Copy link
Contributor Author

Screenshot 2019-06-07 at 15 15 22

@shieldo
Copy link
Contributor

shieldo commented Jun 7, 2019

Interesting - thanks for the report! I hadn't experienced this previously, but certainly anything requested from an entry should be a resolved resource, not just the link.

@ChrisDBrown
Copy link
Contributor Author

https://github.com/usemarkup/contentful/blob/master/src/ResourceBuilder.php#L67
My suspicion is that the issue lies here - based on what's returning I'd expect this to be checking for Type == Link && linkType == Space, as I don't see any instances where getType returns Space

@ChrisDBrown
Copy link
Contributor Author

ChrisDBrown commented Jun 7, 2019

^ going to put up a PR to this effect for review, as it appears to resolve the issue locally, but I'm not 100% clear on the original intent here.

EDIT: retracting this, as it seemed to cause issues elsewhere. It does seem "more correct" to return the Space here than an unresolved Link, but it seems like fixing this is causing issues further downstream.
https://github.com/usemarkup/contentful/blob/master/src/ResourceBuilder.php#L163
Initial thought was to extend this check to also include type "Space", and copy the above code for space handling.

@ChrisDBrown
Copy link
Contributor Author

Just to better illustrate the above: #92

@ChrisDBrown ChrisDBrown mentioned this issue Jun 7, 2019
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

No branches or pull requests

2 participants