foundry icon indicating copy to clipboard operation
foundry copied to clipboard

Support for nested property paths

Open NorthBlue333 opened this issue 3 years ago • 9 comments

Hi, it's me again :wave:

I am currently doing this in my factories:

class CommentFactory {
  public function getDefaults(): array
  {
    return ['user' => UserFactory::new()];
  }

  public function withUserName(): array
  {
    return ['user.name' => 'foo'];
  }

  public function withUserRelation(): array
  {
    // does not work
    return ['user.posts' => PostFactory::new()->many(5)];
  }
}

Which works, I assume thanks to Symfony PropertyAccess component (?). But setting a relation this way does not work because the Factory cannot resolve the relation.

I do not really know how to implement it correctly; also, in some cases I am wondering if the first case can fail (if the object is not set first on the entity in example).

Should this be supported?

My current workaround is to use an afterInstantiate/afterPersist hook.

NorthBlue333 avatar Oct 04 '22 15:10 NorthBlue333

I do not really know how to implement it correctly; also, in some cases I am wondering if the first case can fail (if the object is not set first on the entity in example).

Yeah, I think you're right.

Should this be supported?

My first thought is no just because of the added complexity. We'd have to override the functionality of property access I think.

Either your afterInstantiate hook or using states would be best:

public function withUser(string $name, int $numPosts): self
{
    return $this->addState(['user' => [
        'name' => $name,
        'posts' => PostFactory::new()->many($numPosts),
    ]]);
}

kbond avatar Oct 06 '22 13:10 kbond

Actually after some thinking I thought of this flow for the Factory::create method:

  • Loop on all attributes
  • If the property path is nested, store it in an array keyed by first part of the path
  • Loop again on all non-nested property paths
  • If value is a Factory and property key exists in the array, then add a withAttributes call and unset the entry in the array
  • Apply all nested attributes left in the array just like the other attributes. If it fails, it should be an error user-side.

(Instead of looping we could store differently nested paths in withAttributes method).

What do you think?

I would understand if you do not want to implement it anyway. I made this issue to state it, and if not implemented maybe add it clearly in the docs. :)

NorthBlue333 avatar Oct 07 '22 10:10 NorthBlue333

I think this could work but it would require that the factory have the relation in getDefaults(), no?

kbond avatar Oct 07 '22 10:10 kbond

As stated in my last point, yes, this requires proper use. It might be by setting a default OR by setting the relation with states (as nested properties are always processed after in this flow). Do you see anything else that would result in an error?

NorthBlue333 avatar Oct 07 '22 10:10 NorthBlue333

Do you see anything else that would result in an error?

Could get tricky with embeddables...?

kbond avatar Oct 07 '22 10:10 kbond

Yeah maybe, that is my only concern but actually I think it would also work if it is correctly set up by the user. May I make a PR with the according tests?

NorthBlue333 avatar Oct 07 '22 10:10 NorthBlue333

Sure, if you'd like to experiment with this in a PR, that'd be cool. I am still a little concerned about the added complexity/maintenance burden. I personally prefer a more explicit approach to creating factories.

#306 is close to being ready. Maybe you'd like to wait until that is merged so you can easily run the tests locally?

kbond avatar Oct 07 '22 13:10 kbond

Sure will wait for #306

I understand what you mean. I can make a suggestion, you are free to reject it!

NorthBlue333 avatar Oct 07 '22 14:10 NorthBlue333

hey @NorthBlue333

I'm wondering if this problem could not be fixed using:

class CommentFactory {

  public function withUserAndPosts(): static
  {
    return $this->with(['user' => UserFactory::new(['posts' => PostFactory::new()->many(5))]);
    // OR
    return $this->with(['user' => UserFactory::new()->withPosts(5)]);
  }
}

I'm afraid that supporting nested attributes for relationships would add a lot of complexity with no real added value. But I might be wrong, feel free to open a PR :)

nikophil avatar Jun 21 '24 15:06 nikophil