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

Add support for factory states [enhancement]. Closes #452 #686

Closed
wants to merge 6 commits into from

Conversation

lucacri
Copy link
Contributor

@lucacri lucacri commented Feb 4, 2020

No description provided.

@ScopeyNZ
Copy link
Contributor

ScopeyNZ commented Feb 4, 2020

Hey there. Looks like you've got some pretty opinionated linting on your local machine. You'll have to fix the linting using the patch provided by StyleCI to get the builds green.

@shalvah
Copy link
Contributor

shalvah commented Feb 4, 2020

Thank you for all your help @ScopeyNZ

@@ -193,6 +193,8 @@ public function show($id)
If your controller method uses [Eloquent API resources](https://laravel.com/docs/5.8/eloquent-resources), you can use the apiResource annotations to guide the package when generating a sample response. The `@apiResource` tag specifies the name of the resource. Use `@apiResourceCollection` instead if the route returns a list. This works with both regular `JsonResource` objects and `ResourceCollection` objects.

The `@apiResourceModel` specifies the Eloquent model to be passed to the resource. The package will attempt to generate an instance of the model for the resource from the Eloquent model factory. If that fails, the package will call `::first()` to retrieve the first model from the database. If that fails, it will create an instance using `new`.

The `@apiResourceState` specifies a comma delimited list of states to be passed to the Model Factory.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, the description wasn't that great. @apiResourceState is the docbloc that specifies which factory state to use. It's comma delimited so it can be something like

@apiResourceState delinquent,subscribed

and it will apply the ->states(['delinquent', 'subscribed']) to the factory

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm talking about what I said in the linked comment...? Why go in this direction instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went the way I did because it was easier to implement, and also followed the same syntax of every other docbloc where it never uses =

Copy link
Contributor

Choose a reason for hiding this comment

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

Nah, I don't want us adding another annotation. It's going to be more overhead dealing with the different annotations that we'll end up with—apiResourceState, transformerState etc. I proposed we go with the state= approach because it's easier to extend. For instance, if we're adding model relations, we can add a with= . It might be more work, but it's better in the long run, so I'd like us to move towards parameterized annotations.

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.

3 participants