mirror of
https://github.com/HeyPuter/puter.git
synced 2026-10-13 07:10:05 +00:00
fix: make team deletion resumable, and stop it serializing its mail
`deleteTeam` tore seats down one at a time, each one awaiting a mail send, so a full 40-seat team was 40 round trips end to end inside a single request. The per-seat work is independent, so it now runs under the same bounded-concurrency helper the rest of the backend uses for fan-out sends. The loop also had no equivalent of the guard `disableMember` carries. That one checks `suspended` before emitting, with the reason written down: a second `team.account.disabled` opens a second byte charge that only one `enabled` will ever close. Deleting a team emitted unconditionally, so any partial failure followed by a retry double-charged every seat the first attempt had already disabled. The seat teardown moves into one method both paths can state the same way, and a retry now resumes instead of re-charging. Failure semantics are unchanged: a seat that throws still aborts the delete and leaves the team live, which is what makes the retry meaningful. Touching this file reformats a few untouched spans -- it carried ten pre-existing `prettier/prettier` errors that the repo's own pre-commit `eslint --fix` resolves on any commit that stages it.
This commit is contained in:
1 parent
665314535e
commit
d896250ae2
2 files changed
+83
-38
No files matched your search
@@ -191,6 +191,24 @@ describe('team billing events', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('does not re-charge a seat that was already disabled before the delete', async () => {
|
||||
const team = await makeTeam();
|
||||
const a = await provision(team);
|
||||
const b = await provision(team);
|
||||
await service.disableMember(team.uid, owner.id, a.userId);
|
||||
seen.length = 0;
|
||||
|
||||
await service.deleteTeam(team.uid, owner.id);
|
||||
|
||||
// Only one `enabled` will ever close a charge, so only one may open it.
|
||||
const disabled = of('team.account.disabled');
|
||||
expect(disabled).toHaveLength(1);
|
||||
expect(disabled[0].data.user_id).toBe(b.userId);
|
||||
|
||||
// Still both seats: the team is losing two accounts either way.
|
||||
expect(of('team.deleted')[0].data).toMatchObject({ account_count: 2 });
|
||||
});
|
||||
|
||||
it('emits account-deleted, which a post-delete listener could not', async () => {
|
||||
const team = await makeTeam();
|
||||
const seat = await provision(team);
|
||||
|
||||
@@ -28,11 +28,15 @@ import { isReservedUsername } from '../../util/reservedUsernames.js';
|
||||
import type { EmailTemplateName } from '../../clients/email/templates.js';
|
||||
import { subscriptionSatisfies } from '../metering/enforcement.js';
|
||||
import { ORG_SEAT_FREE_SUBSCRIPTION } from '../metering/consts.js';
|
||||
import { runWithConcurrencyLimit } from '../../util/concurrency.js';
|
||||
|
||||
// A free team is small on purpose; paying widens it. Both overridable in config.
|
||||
const FREE_SEAT_CAP = 4;
|
||||
const PAID_SEAT_CAP = 40;
|
||||
|
||||
// Seat teardown is independent per member, so a delete need not be serial.
|
||||
const TEARDOWN_CONCURRENCY = 8;
|
||||
|
||||
import type {
|
||||
EventMap,
|
||||
TeamBillingContext,
|
||||
@@ -637,32 +641,19 @@ export class TeamService extends PuterService {
|
||||
// Otherwise they keep working, unreachable through a deleted team.
|
||||
let page = await this.stores.team.listMembers(teamUid, { limit: 200 });
|
||||
for (;;) {
|
||||
for (const member of page.items) {
|
||||
if (Number(member.org_owned) !== 1) continue;
|
||||
await this.stores.team.appendAudit({
|
||||
teamId: team.id,
|
||||
userId: member.user_id,
|
||||
actorUserId,
|
||||
action: 'disable',
|
||||
reason: 'team_deleted',
|
||||
});
|
||||
const held = await this.#heldBytes(member.user_id);
|
||||
await this.#suspend(member.user_id);
|
||||
disabled++;
|
||||
|
||||
// Per seat, not one bulk event: the byte charge is per account.
|
||||
this.#emitBilling('team.account.disabled', {
|
||||
...billing,
|
||||
user_id: member.user_id,
|
||||
user_uuid: member.uuid,
|
||||
username: member.username,
|
||||
held_bytes: held,
|
||||
});
|
||||
|
||||
// The team notice covers the disabling, so a member is
|
||||
// told once rather than twice about the same event.
|
||||
await this.#notifyMember(member.user_id, 'team_closed', team);
|
||||
}
|
||||
// Bounded: a seat costs a mail send, and a full team is 40 of them.
|
||||
const outcomes = await runWithConcurrencyLimit(
|
||||
page.items.filter((m) => Number(m.org_owned) === 1),
|
||||
TEARDOWN_CONCURRENCY,
|
||||
(member) =>
|
||||
this.#disableSeatForDelete(
|
||||
member,
|
||||
team,
|
||||
actorUserId,
|
||||
billing,
|
||||
),
|
||||
);
|
||||
disabled += outcomes.filter(Boolean).length;
|
||||
if (!page.cursor) break;
|
||||
page = await this.stores.team.listMembers(teamUid, {
|
||||
limit: 200,
|
||||
@@ -684,6 +675,44 @@ export class TeamService extends PuterService {
|
||||
});
|
||||
}
|
||||
|
||||
/** As `disableMember`; a suspended seat is skipped so a retry resumes. */
|
||||
async #disableSeatForDelete(
|
||||
member: TeamMemberRow,
|
||||
team: TeamRow,
|
||||
actorUserId: number,
|
||||
billing: TeamBillingContext,
|
||||
): Promise<boolean> {
|
||||
const current = await this.stores.user.getByProperty(
|
||||
'id',
|
||||
member.user_id,
|
||||
{ force: true },
|
||||
);
|
||||
if (current?.suspended) return true;
|
||||
|
||||
await this.stores.team.appendAudit({
|
||||
teamId: team.id,
|
||||
userId: member.user_id,
|
||||
actorUserId,
|
||||
action: 'disable',
|
||||
reason: 'team_deleted',
|
||||
});
|
||||
const held = await this.#heldBytes(member.user_id);
|
||||
await this.#suspend(member.user_id);
|
||||
|
||||
// Per seat, not one bulk event: the byte charge is per account.
|
||||
this.#emitBilling('team.account.disabled', {
|
||||
...billing,
|
||||
user_id: member.user_id,
|
||||
user_uuid: member.uuid,
|
||||
username: member.username,
|
||||
held_bytes: held,
|
||||
});
|
||||
|
||||
// The team notice covers the disabling, so a member is told once.
|
||||
await this.#notifyMember(member.user_id, 'team_closed', team);
|
||||
return true;
|
||||
}
|
||||
|
||||
/** Team owner only. Readable after deletion -- that is the point of it. */
|
||||
async listAudit(
|
||||
teamUid: string,
|
||||
@@ -820,18 +849,16 @@ export class TeamService extends PuterService {
|
||||
id === null ? null : (users.get(id)?.username ?? null);
|
||||
|
||||
return {
|
||||
items: page.items.map(
|
||||
(row): MemberActivityEntry => ({
|
||||
action: row.action,
|
||||
reason: row.reason,
|
||||
created_at: epochSeconds(row.created_at),
|
||||
username: name(row.user_id_keep),
|
||||
actor_username: name(row.actor_user_id),
|
||||
// Only a sign-in carries these; the shape stays uniform.
|
||||
ip: null,
|
||||
user_agent: null,
|
||||
}),
|
||||
),
|
||||
items: page.items.map((row): MemberActivityEntry => ({
|
||||
action: row.action,
|
||||
reason: row.reason,
|
||||
created_at: epochSeconds(row.created_at),
|
||||
username: name(row.user_id_keep),
|
||||
actor_username: name(row.actor_user_id),
|
||||
// Only a sign-in carries these; the shape stays uniform.
|
||||
ip: null,
|
||||
user_agent: null,
|
||||
})),
|
||||
...(page.cursor ? { cursor: page.cursor } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user