Closed AdamEsterle closed 7 years ago
Pasting original response from https://github.com/nothingworksinc/ticketbeast/commit/68a1d94540a09ef1bcfaa6cde21ce129ba59d1af#commitcomment-22223091
Hey @AdamEsterle!
The reason for not using a model factory for forms is just that the form fields and the model fields are not the same. For example, the form has a date
field and a time
field that are combined into a single date
column on the model. There's also the ticket_quantity
field that doesn't map to any column in the database either.
In general, any time I see a form and a model having the exact same fields, I view it as a coincidence, not as actual duplication that should be backed by a single abstraction. I don't like to introduce coupling like that between the UI and the database.
I've seen other folks use model factories to populate a POST request payload and it's always felt really wrong to me. It feels like going out of your way to reuse something in an awkward way that isn't even tied to the form; again it's really just coincidence that your form and database happen to be the same. The form fields and database columns can definitely change at different rates, in different ways, and for different reasons, so I don't think it's a good idea to back them with the same abstraction.
There's also a ton of endpoints in applications that aren't related to models at all, or even more different than this concerts example. Our concerts/{id}/orders
end point is a good one, here's what the POST payload looks like:
[
'email' => 'john@example.com',
'ticket_quantity' => 3,
'payment_token' => $this->paymentGateway->getValidTestToken(),
]
...and here's what the orders
table looks like:
Schema::create('orders', function (Blueprint $table) {
$table->increments('id');
$table->string('confirmation_number');
$table->integer('amount');
$table->string('email');
$table->string('card_last_four');
$table->timestamps();
});
Almost nothing in common! It's still very useful to have a way to generate valid parameters for this endpoint though, so having a form factory helper here could be really helpful.
Hope that makes sense!
I think "Issues" is probably a better place for questions for now, because it'll be easier for other folks to browse them after the fact 👍🏻
@adamwathan I really like this approach! Nice and clean
Question: Why not just use factory()->make()? Since the factory will already be returning a model with valid data, you can save yourself from having to make a "validParams" method for every different model and its endpoint.
Then just do
$this->post(/endpoint, factory()->make(['override' => 'override])->toArray());
Thoughts: Valid Params would be better for endpoints where a whole model is not needed (say a form is just updating the title of the concert with not other fields). But then you could just do
$this->post(/endpoint, collect(factory()->make(['override' => 'override])->toArray())->only('override'));
^^ That is rather long, but it could be wrapped in a helper function to be more expressiveBUT - that might not even be necessary since you can pass 5 fields to an /endpoint that only needs 1 and the excess is automatically disregarded by Laravel anyways (unless you are using request->all()). So perhaps factory()->make() is really all you need???
Adam Wathan posted a response here: https://github.com/nothingworksinc/ticketbeast/commit/68a1d94540a09ef1bcfaa6cde21ce129ba59d1af#commitcomment-22223091