Adds Form Request for Creating Departments

[spencerrlongg]

This adds a form request for the store department method in the API. For some reason watson\validating wasn't picking up that there was an array in a request which resulted in a 500, this ensures that's caught before we do the ->fill(...).

Adds tests for this as well. For testing errors, ->assertOk (which asserts a 200 status), and assertStatusMessageIs('error') in combination more or less assert one of our validation errors.

[spencerrlongg]

Moving to draft so I can add in some assertions for the db to ensure values are being stored properly.

[spencerrlongg]

Added DbHas assertion

[spencerrlongg]

Also, those attributes need to be added to the API docs as currently only name is there.

[snipe]

We probably need to allow this for users who can edit departments as well?

[spencerrlongg]

This is just the store request, update request would be separate

[snipe]

I'm not sure a form request is the right move here tbh

[spencerrlongg]

I'm not sure, I can see why you'd say that, but I'm still of the mind that form requests are generally good and an improvement in terms of both security and DX (which is obviously subjective).

Security wise in this one I don't like that we were filling with a $request->all() - sure filling should be gated by the fillable property on the model itself, but it still makes me nervous. Filling with $request->validated() absolutely ensures that there's nothing getting into the controller that isn't explicitly already validated, even if the validation is as simple as string.

Next, I can't figure out why watson wasn't catching that values were arrays in the first place, is it because we were filling and filling is doing something db wise now? Was save actually happening before watson? Neither should be the case. Either way, troubleshooting that sounded like I was going to go down a rabbit hole of Xdebug so I went ahead and decided that the tried and true method of form requests was the way to go. Also, this issue is re-creatable. (https://app.shortcut.com/grokability/story/29245/illuminate-database-queryexception-array-to-string-conversion-connection-mysql-sql-insert-into-departments-name-company-id)

I still think that we should continue expanding our form requests to near every request, so as I see issues like this pop up I'm generally going to go that route.

Shortcut

[snipe]

I still think that we should continue expanding our form requests to near every request, so as I see issues like this pop up I'm generally going to go that route.

Form requests get pretty gnarly once you start trying to handle bulk actions though. I think they're obviously very useful, but they're not a silver bullet and cause a giant pain when you're trying to validate bulk stuff with any level of nuance.

[spencerrlongg]

I agree that it can get a little messy in a couple situations - I did say "near every request".

On bulk things, I think most of the time the only real difference between a bulk request and a singular request is the ids array, so really it'd just be adding a conditional ids validation based on the URL which we can pretty easily get at by with $this->url()...