Skip to content

Backport #1014 (also includes #907)#1097

Merged
dunglas merged 2 commits into
api-platform:2.0from
teohhanhui:hotfix/backport-1014
May 15, 2017
Merged

Backport #1014 (also includes #907)#1097
dunglas merged 2 commits into
api-platform:2.0from
teohhanhui:hotfix/backport-1014

Conversation

@teohhanhui

Copy link
Copy Markdown
Contributor
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets N/A
License MIT
Doc PR N/A

Since it's not feasible to backport #1014 without #907, I've included both. Basically just copy-pasted these files from master \o/

@teohhanhui
teohhanhui requested review from dunglas and soyuka May 4, 2017 07:03
@teohhanhui
teohhanhui changed the base branch from master to 2.0 May 4, 2017 07:03
$doctrineClassMetadata = $manager->getClassMetadata($resourceClass);
$doctrineIdentifierFields = $doctrineClassMetadata->getIdentifier();
$isOrm = interface_exists(EntityManagerInterface::class) && $manager instanceof EntityManagerInterface;
$platform = $isOrm ? $manager->getConnection()->getDatabasePlatform() : null;

@soyuka soyuka May 4, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

phpstan isn't happy with this :| We added the following in phpstan.neon on master:

        - '#Call to an undefined method Doctrine\\Common\\Persistence\\ObjectManager::getConnection\(\)#'

@teohhanhui
teohhanhui force-pushed the hotfix/backport-1014 branch from c431be1 to 40798a1 Compare May 5, 2017 04:19
@teohhanhui

teohhanhui commented May 5, 2017

Copy link
Copy Markdown
Contributor Author

@dunglas Behat tests failed for me (the same 7 scenarios failed, like PHP 7.0 on Travis) on my local PHP 7.1.4 environment as well (using api-platform Docker php image). I'm afraid it's a problem with our code.

@soyuka

soyuka commented May 5, 2017

Copy link
Copy Markdown
Member

@teohhanhui check #1099 (comment)

@teohhanhui

Copy link
Copy Markdown
Contributor Author

@soyuka There is another failure:

    And the JSON node "_embedded.item" should have 1 element
Behatch\Context\JsonContext::theJsonNodeShouldHaveElements()
      Failed to evaluate expression '_embedded.item' (Exception)

@meyerbaptiste

Copy link
Copy Markdown
Member

This problem is resolved by #1102.

@teohhanhui
teohhanhui force-pushed the hotfix/backport-1014 branch from 40798a1 to 1265798 Compare May 15, 2017 10:23
@teohhanhui

Copy link
Copy Markdown
Contributor Author

Rebased.

ping @api-platform/core-team

@soyuka soyuka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nice, we should merge this 👍

@dunglas
dunglas merged commit c902817 into api-platform:2.0 May 15, 2017
@dunglas

dunglas commented May 15, 2017

Copy link
Copy Markdown
Member

Thanks @teohhanhui

@teohhanhui
teohhanhui deleted the hotfix/backport-1014 branch May 15, 2017 12:03
hoangnd25 pushed a commit to hoangnd25/core that referenced this pull request Feb 23, 2018
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.

5 participants