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

Es add entity set #12

Open
wants to merge 2 commits into
base: 8.x-1.0
Choose a base branch
from
Open

Es add entity set #12

wants to merge 2 commits into from

Conversation

ericgsmith
Copy link

This includes the set entity method that is on the v3.1 branch.

The previous test fails on Drupal 8.3 as the php unit autoload logic has changed. I have manually loaded the file - but I am not sure if this is the correct approach.

In drupal_phpunit_get_extension_namespaces function the logic has changed so the autoload will only recognise the following directories $suite_names = ['Unit', 'Kernel', 'Functional', 'FunctionalJavascript'];

I can't find examples of how to load in our additional Fixtures directory - however this will for know at least stop the test from failing.

@ericgsmith ericgsmith requested review from itarato and mike1937 May 4, 2017 10:11
@itarato
Copy link
Contributor

itarato commented May 4, 2017

@ericgsmith Hey homie, I realized why I don't like this code. The entity controller suppose to be a non volatile container, and you exposing the entity setter opens the danger of anyone changing the content anytime. Please - unless you have a better idea - add a check in the setEntity method to verify that the entity given has exactly the same type-bundle-id triplet. If not, better to throw an exception or reject it.

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

Successfully merging this pull request may close these issues.

2 participants