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

Allow Entities and Array of Entities #43

Closed
pabloelcolombiano opened this issue Jan 22, 2021 · 9 comments
Closed

Allow Entities and Array of Entities #43

pabloelcolombiano opened this issue Jan 22, 2021 · 9 comments
Assignees
Labels
enhancement New feature or request

Comments

@pabloelcolombiano
Copy link
Collaborator

For the moment, the make method allows only arrays, callable, and int.

This should be extended to entity interfaces and arrays of entity interfaces.

Important is that when performing a ->toArray() on an entity, hidden fields are not "rendered" in the array.

So performing a ->toArray() will not do, unless we call ->setHidden([]) prior to the ->toArray().

If you take a look at the Article.php file, there is extra a hidden array to test that this case is covered. This should be covered by the tests and the implementation.

I propose to create a new TestCase\Factory\BaseFactoryMakeWithEntity.php or so class with the tests covering this new feature.

@pabloelcolombiano
Copy link
Collaborator Author

pabloelcolombiano commented Jan 22, 2021

Thinking about it again:

For the moment, the Factory works with arrays of data, and I mentioned above calling ->toArray() on the entities passed in order to work as usual with arrays.

But it might be more interesting to work with entities, and perform a $this->getTable()->newEntity($makeParameter)or $this->getTable()->newEntities($makeParameter) on the array passed as parameter, to have the factories working per se with entities. I think we can take more out of CakePHP from that, and overcome the issue ->toArray()brings with hidden fields.

The BaseFactory::getEntity() resp. BaseFactory::getEntities() become then obsolete, we keep them, but they do even less.

The DataCompiler::compileEntity() returns a real Entity, or array of Entities, and not an array anymore.

If we achieve that, I pay a round of beers!

@pabloelcolombiano
Copy link
Collaborator Author

pabloelcolombiano commented Jan 23, 2021

The package now works with entities, and not with arrays (#54)

This is pushed on dev.

What is left to do is to allow the dev to feed the Factories with entities and arrays of entities.

@pabloelcolombiano
Copy link
Collaborator Author

Note that the ->with() method also should be feedable with Entities.

@vidrascus
Copy link
Collaborator

We are supporting now Entities and Array of Entities

vidrascus pushed a commit that referenced this issue Jan 25, 2021
* support Entities and Array of Entities
@pabloelcolombiano
Copy link
Collaborator Author

pabloelcolombiano commented Jan 26, 2021

Thanks for the contribution, it fits the requirements.

  • Since the developer is feeding the factory with an entity and the DataCompiler now works internally with entities, we can instead of performing a ->toArray() on that entity as you did in the compileEntity method, directly make use of that entity and let the DataCompiler build over that entity.

You can have a look at the commit above, and let me know if this fits from your point of view.

@pakacuda
Copy link
Collaborator

Ok, can we merge Pablo’s branch (https://github.com/vierge-noire/cakephp-fixture-factories/tree/%2343_allow_entities_and_array_of_entities) in the next version and move on ?
@pabloelcolombiano : can I create the pull request and merge it as it is ?
AFAIK, Pablo’s commits were done on top of the next branch (cakephp 4) and we also have to port that the cake3_next one, correct ?

@vidrascus
Copy link
Collaborator

Yes, true. We also have to port the implementation to the cake3_next branch.

@pabloelcolombiano
Copy link
Collaborator Author

@pakacuda I have released a cake3 compatible implementation (simple cherry-pick). I'll merge both cake4 and cake3 implementations in the next- branches in a minute.

@pabloelcolombiano
Copy link
Collaborator Author

This is done, I close the issue for now.

Thanks for constructive work together. This is a really nice feature, probably the last before the v2.2 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants