fix: repair four defects in the never-executed application layer - #11
Merged
Merged
Conversation
These use cases are written and complete-looking, and none of them works. Nothing has ever called them, so nothing has ever found out. **Membership changes now require an authorized actor.** AddTeamMemberUseCase and AddProjectMemberUseCase checked that the target was not already a member and never considered who was asking. The routes carry AuthMiddleware, which answers "is this someone", not "may this someone change this membership" -- so once wired, any logged-in user could have added any account, at role 'admin', to any team or project. Both now take a required $actorId and refuse unless that user is already an admin of the group. $actorId is deliberately not defaulted: a default would let a caller omit an authorization decision without noticing. **CreateTeamUseCase could not insert a row.** It wrote `created_by`. `teams` has `owner_id NOT NULL` and no `created_by` column, and Database::insert() builds its column list straight from the array keys, so MySQL rejected the statement. **CreateWorkspaceUseCase failed three ways.** Same wrong column, plus `owner_id NOT NULL` and `slug VARCHAR(200) NOT NULL UNIQUE` were never set and nothing generated a slug. It now sets the owner and derives a slug from the name with a random suffix -- the column is unique, so a collision is a failed insert, and a random suffix is cheaper and more predictable than a retry loop for a value that is not user-facing. **Creating a workspace left it invisible to its creator.** CreateTeamUseCase and CreateProjectUseCase both call addMember(..., 'admin') after inserting; this one did not, and UserRepository::getWorkspaces() reads through workspace_members. WorkspaceRepository::addMember() already existed. Reported with evidence in #10. The one remaining item there is the three workspace methods with no application layer at all, which needs decisions rather than repair. The signature changes are safe: these use cases still have no callers. Not verified locally: no PHP on this machine. phpcs and phpstan run in CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes 1–4 of #10.
These use cases are written, complete-looking, and none of them works. Nothing
has ever called them, so nothing has ever found out.
1. Membership changes now require an authorized actor
AddTeamMemberUseCaseandAddProjectMemberUseCasechecked that the targetwas not already a member, and never considered who was asking.
AuthMiddlewareanswers is this someone, not may this someone change thismembership. Wired as they stood, any logged-in user could have added any
account — including their own, at
role = 'admin'— to any team or project.Both now take a required
$actorIdand refuse unless that user is already anadmin of the group. It is deliberately not defaulted: a default would let
a caller omit an authorization decision without noticing.
2.
CreateTeamUseCasecould not insert a rowIt wrote
created_by.teamshasowner_id INT UNSIGNED NOT NULLand nocreated_bycolumn, andDatabase::insert()builds its column list straightfrom the array keys — so MySQL rejected the statement outright.
3.
CreateWorkspaceUseCasefailed three ways at onceSame wrong column, plus
owner_id NOT NULLandslug VARCHAR(200) NOT NULL UNIQUEwere never set, and nothing in thecodebase generated a slug.
It now sets the owner and derives a slug from the name with a random suffix.
The column is unique, so a collision is a failed insert rather than a cosmetic
problem — and for a value that is not user-facing, a random suffix is cheaper
and more predictable than a query-and-retry loop.
4. A new workspace was invisible to the person who created it
CreateTeamUseCaseandCreateProjectUseCaseboth calladdMember(..., 'admin')after inserting. This one did not, andUserRepository::getWorkspaces()reads throughworkspace_members:So even with 2 and 3 fixed, the creator would not have seen it on their own
dashboard.
WorkspaceRepository::addMember()already existed.Not fixed here
Item 5 of #10 —
WorkspaceController::update/addMember/removeMemberhave noapplication layer at all. That needs decisions about who may change a
workspace, not repair.
Safety
The two signature changes are safe: these use cases still have no callers, so
nothing can be silently passing the old argument list.
Not verified locally — no PHP on this machine. phpcs and phpstan level 5
run here; behaviour stays uncovered until #2.
🤖 Generated with Claude Code