Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .ai/guidelines/app.actions.blade.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
- Inject dependencies via constructor using private properties.
- Create new actions with `php artisan make:action "{name}" --no-interaction`
- Wrap complex operations in `DB::transaction()` within actions when multiple models are involved.
- Transactions and row locks that guard a write live in actions, never in HTTP middleware: that puts database rules in the HTTP layer and holds the transaction open for the whole request.
Comment thread
mnapoli marked this conversation as resolved.
Resolved
- When a concurrent request can pass the same check before the write (a limit, the last admin), the action locks the parent row (the conversation, the organization) in a short `DB::transaction()`, checks the condition again under the lock, and doesn't write if it no longer holds. This is not validation: it never aborts or throws a `ValidationException` or an HTTP exception, and the request keeps its own check and error message. When the caller must tell the user, `throw new` a dedicated exception from `app/Exceptions` (never `throw_if()`), named for the rule it protects, without an `Exception` suffix, and the caller catches it (`OrganizationMustHaveAnAdmin`). `CreateOrganizationAdmin`, which returns `false` instead, predates this. When the request's error is enough, skip the write silently (`UpdateMessagePin`).
- Some actions won't require dependencies via `__construct` and they can use just the `handle()` method.
- Actions never authorize or validate anything: that lives in the form request of the endpoint calling them.
- Actions never authorize or validate anything: that lives in the form request of the endpoint calling them. Checking again under a lock, above, is the only exception.

## CRUD by design

Expand Down
2 changes: 1 addition & 1 deletion .ai/guidelines/app.http.requests.blade.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

- Never use policies or gates: no `app/Policies`, no `Gate::define()`, no `can:` middleware, no `$user->can()`, no `$this->authorize()`.
- Authorization and validation only ever happen in form requests: authorization in `authorize()`, validation in `rules()`. Never in controllers, actions, or models. This includes `index` and `show` endpoints.
- Controllers and actions never check anything themselves: no `abort()`, `abort_if()`, `abort_unless()`, `throw_if()`, `throw_unless()`, `Validator::make()`, `$request->validate()`, or `ValidationException`.
- Controllers and actions never check anything themselves: no `abort()`, `abort_if()`, `abort_unless()`, `throw_if()`, `throw_unless()`, `Validator::make()`, `$request->validate()`, or `ValidationException`. The only exception is an action checking again under a row lock to close a race, which skips the write or throws its own exception for the caller to catch (see the actions guidelines).
- Create a dedicated form request for each endpoint that checks anything, and never share one between endpoints. If the endpoint has none yet, create one.
- Name it after the controller method it serves, CRUD by design: `index` → `List…Request`, `show` → `Show…Request`, `store` → `Create…Request`, `update` → `Update…Request`, `destroy` → `Delete…Request`. For example `CommitPinController@update` → `UpdateCommitPinRequest`, `CommitPinController@destroy` → `DeleteCommitPinRequest`, `CommitApprovalController@store` → `CreateCommitApprovalRequest`.
- When hiding something should look like it does not exist, return `Response::denyAsNotFound()` from `authorize()` instead of `false`.
Expand Down
Loading