A client-scoped staff member with create_clients created a client and lost it in the same request. guardTarget() answers 404 for anything off their roster, and StaffLibraryScope::clients() leaves it out of their list -- so the record existed, was logged, was welcomed by email, and was invisible to the person who made it. store() redirects to the edit page, which is exactly where they landed on a 404. The API twin had the same shape: a scoped token got a 404 from every route that binds the client it had just created.
The new client is now attached to the creator's roster when the creator is client-scoped, on both sides. That is where a client they created belongs -- the roster is the same list assignedClients already uses for everything else they may reach. Unscoped creators gain nothing: they see every client already, and a roster entry would change what assignedClients means for them. Nothing is attached retroactively.
The widening this involves is self-limited: the only thing added is an account the creator just made, which starts with no files, no folders and no group memberships, so assignableClientIds gains nothing to reach. Seats do not move either, since they are counted from active and account_requested.
Verified before merging: 34 passed across both suites on the trial-merge, 2 failed / 32 passed with app/ reset. This is the busiest file set of the series -- it shares ClientsController with #1718 and Api/ClientsController plus the API test file with #1723 -- so the merged result was read rather than trusted: #1718's reassign_candidates gating and #1723's patchCustomFieldValues are both intact alongside it. scramble:export reproduces the committed docs/api/openapi.json byte for byte.
Reported and fixed by @denkfabrik-li.
A client-scoped staff member with create_clients creates a client and
loses it immediately. guardTarget() answers 404 for anything off their
roster, and StaffLibraryScope::clients() leaves it out of their list -- so
the record exists, is logged, is welcomed by email, and is invisible to
the person who made it. store() redirects to the edit page, which is where
they land:
POST /clients → 302 → /clients/4
on_creator_roster → false
GET /clients/4/edit → 404
clients listed → ["Mine"] the new client is not there
Their own roster is where a client they created belongs, so it is attached
there. An unscoped creator gains nothing: they see every client already,
and a roster entry would change what assignedClients means for them.
The API twin does the same, for the same reason -- a scoped token gets a
404 from every route that binds the client it just created.
Three tests: the scoped creator can open and list the client, an unscoped
creator gains no roster entry, and the API twin behaves like the web. The
first and third go red without the fix.
update() states the rule eighteen lines above the bug: "PATCH semantics,
unlike the web form which always submits every field: an absent key means
'leave alone', not 'clear'." Every column obeys it. The custom fields did
not, because they went through create()'s pass, which walks every field
there is and writes null for the ones the request did not carry.
Two fields filled, a PATCH naming one:
status → 200
named field → "Robin"
the other one → null (was "ATU12345678")
Nothing says so in the response, and there is no other copy of the value.
The write pass is now shared but entered two ways: create() keeps writing
every field, since a new client has no values and a checkbox nobody ticked
is a recorded "no"; update() writes only the fields the request named.
Three tests: the untouched field survives, a named empty value still
clears, and create still records every field. Without the fix the first
goes red.
Deleting a staff or client account is two writes: soft-delete the account,
then cascade or reassign the files and folders it owns. All four destroy()
paths (Users + Clients, web + API) ran them one after the other with nothing
tying them together.
If the second write throws, the account is already gone but its content is
not handled. The concrete way in is the reassign branch: validate() checks
reassign_to_id with exists(active), but apply() re-resolves it with
findOrFail() a moment later (AccountContentDeletion:108), so a target
deactivated or deleted in between throws — leaving a soft-deleted account
whose files still point at it, and a UserDeleted log for a deletion that did
not finish.
Wrap the delete()+apply() pair in a single DB::transaction() in each of the
four destroy() methods. validate() and the authorization guards stay outside
it: they are read-only and must be able to reject before anything is written.
cascadeDelete()/reassignTo() already open their own transaction, which nests
as a savepoint under this one, so the account soft-delete, its activity log,
and the content work now commit or roll back together.
Tests: a DeletedAccountContent double that reports content to handle and then
throws while handling it (tests/Helpers.php) drives one test per destroy()
endpoint asserting the account survives the failure and no UserDeleted entry
is written; each goes red against the un-wrapped controller.
Client file sharing, rebuilt from the ground up: a private area per
client, resumable uploads, folders, groups and categories, sharing with
expiry dates and download limits, comments, file versions, an activity
log, a REST API, and sixteen languages.
This repository begins here. ProjectSend 2 was developed privately, and
that development history is not published — the previous generation
remains available, with its own history, at projectsend/legacy.
Free software under the GNU General Public License v2, or (at your
option) any later version.
ignacionelson