Skip to content

Commit 6ffe459

Browse files
committed
chore(webapp): trim comments in directorySyncEffects
Condense the kept rationale comments (logLevel/warn, last-Owner dedup, role overwrite) and drop the obvious function-header comments that just restated the code. No behavior change.
1 parent e57fd9c commit 6ffe459

1 file changed

Lines changed: 7 additions & 19 deletions

File tree

apps/webapp/app/services/directorySyncEffects.server.ts

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,15 @@ import { createPlatformNotification } from "~/services/platformNotifications.ser
1111

1212
const LAST_OWNER_NOTIFICATION_TITLE = "Directory Sync: last Owner protected";
1313

14-
// Directory-sync effects are idempotent and the accounts-webhook worker retries
15-
// the whole event (maxAttempts: 5). A single failed attempt is therefore an
16-
// expected, self-healing condition rather than an alert-worthy error — the most
17-
// common case is a role assignment losing a serializable race during a backfill
18-
// burst (the plugin already retries that internally; the worker retry mops up
19-
// the rest). Tag the thrown error with `logLevel: "warn"` so the worker logs it
20-
// at warn instead of error (see redis-worker `processItem`), keeping it visible
21-
// for triage without paging as an error on every transient retry.
14+
// Effects are idempotent and the worker retries, so a single failed attempt is
15+
// transient (usually a serializable-conflict retry), not an alert. `logLevel`
16+
// makes the worker log it at warn instead of paging.
2217
function retryableEffectError(message: string): Error {
2318
return Object.assign(new Error(message), { logLevel: "warn" as const });
2419
}
2520

26-
// Raise a user-scoped, deduped notification when the directory tried to
27-
// remove the org's last Owner. We keep the member and tell the Owner what to
28-
// do; a single undismissed notification is enough (don't spam on every retry).
21+
// Deduped notification when the directory tried to remove the org's last Owner:
22+
// we keep the member, and one undismissed notification is enough (no retry spam).
2923
async function notifyLastOwnerProtected(userId: string, organizationId: string): Promise<void> {
3024
const existing = await prisma.platformNotification.findFirst({
3125
where: {
@@ -72,9 +66,6 @@ async function notifyLastOwnerProtected(userId: string, organizationId: string):
7266
}
7367
}
7468

75-
// Apply one directory-sync membership effect against public.* tables. The
76-
// plugin owns all enterprise.* state and never writes here; this is the only
77-
// path that mutates User / OrgMember / roles / tokens from a directory event.
7869
async function applyEffect(effect: DirectorySyncEffect): Promise<void> {
7970
switch (effect.kind) {
8071
case "provision": {
@@ -95,8 +86,8 @@ async function applyEffect(effect: DirectorySyncEffect): Promise<void> {
9586
source: "directory_sync",
9687
});
9788

98-
// Directory is authoritative for role: overwrite even for an existing
99-
// member (ensureOrgMember only sets the role on first create).
89+
// Directory owns the role: overwrite even an existing member
90+
// (ensureOrgMember only sets it on create).
10091
if (effect.roleId) {
10192
const result = await rbac.setUserRole({
10293
userId,
@@ -133,9 +124,6 @@ async function applyEffect(effect: DirectorySyncEffect): Promise<void> {
133124
}
134125
}
135126

136-
// Apply all effects from a processed directory-sync webhook. Effects are
137-
// idempotent, so a worker retry that re-applies them converges. A throw
138-
// propagates to the worker for retry.
139127
export async function applyDirectorySyncEffects(effects: DirectorySyncEffect[]): Promise<void> {
140128
for (const effect of effects) {
141129
await applyEffect(effect);

0 commit comments

Comments
 (0)