fix(deferred): address /code-review findings on the closeout diff
- channels CB3: sort discovery rows NULLs-last (title is None) to match the DB
ORDER BY exactly — coercing NULL title to "" floated untitled channels to the top.
- AdminUsers SC1: track in-flight rows in a Set, not a single id — per-row disabling
now lets the admin start concurrent row actions, and a shared id was cleared by
whichever settled first (re-enabling a still-pending row → possible double-submit).
- ChatThread CT4: reset the incoming-count ref when partnerId changes — the Messages
page reuses one ChatThread across conversation switches, so a same-count switch
could skip the open-marks-read badge refresh.
- messages MB3: push a bare {type:"unread"} (drop the now-dead count query — the
client re-fetches on the signal and ignored the number anyway).
- api.ts: extract the shared keepalive `beacon()` helper (saveProgressBeacon +
plexProgressBeacon were near-verbatim copies).
This commit is contained in:
parent
f0198f2e0a
commit
3394dabbeb
5 changed files with 72 additions and 64 deletions
|
|
@ -185,7 +185,11 @@ def discover_channels(
|
||||||
with quota.attribute(user.id, quota.QuotaAction.CHANNELS_DISCOVER), YouTubeClient(db, user) as yt:
|
with quota.attribute(user.id, quota.QuotaAction.CHANNELS_DISCOVER), YouTubeClient(db, user) as yt:
|
||||||
apply_channel_details(db, yt.get_channels(need))
|
apply_channel_details(db, yt.get_channels(need))
|
||||||
db.commit()
|
db.commit()
|
||||||
rows = sorted(rows, key=lambda r: (-int(r[1]), (r[0].title or "").lower()))
|
# Match _discovery_rows' ORDER BY exactly: video-count DESC, then title (NULLs LAST,
|
||||||
|
# as Postgres orders them — coercing NULL→"" would wrongly float untitled channels up).
|
||||||
|
rows = sorted(
|
||||||
|
rows, key=lambda r: (-int(r[1]), r[0].title is None, (r[0].title or "").lower())
|
||||||
|
)
|
||||||
except YouTubeError as exc:
|
except YouTubeError as exc:
|
||||||
log.warning("discovery: channel enrich failed (user %s): %s", user.id, exc)
|
log.warning("discovery: channel enrich failed (user %s): %s", user.id, exc)
|
||||||
db.rollback()
|
db.rollback()
|
||||||
|
|
|
||||||
|
|
@ -319,15 +319,10 @@ def get_thread(
|
||||||
changed = True
|
changed = True
|
||||||
if changed:
|
if changed:
|
||||||
db.commit()
|
db.commit()
|
||||||
# MB3: tell this user's OTHER tabs/devices their unread dropped so the badge updates
|
# MB3: ping this user's OTHER tabs/devices that their unread dropped so the badge
|
||||||
# live instead of waiting for the next 20s poll. Carries the fresh total so the client
|
# refreshes live instead of waiting for the next 20s poll. The client re-fetches the
|
||||||
# needs no extra fetch.
|
# count on this signal, so no number is carried here.
|
||||||
n = db.scalar(
|
manager.push(user.id, {"type": "unread"})
|
||||||
select(func.count())
|
|
||||||
.select_from(Message)
|
|
||||||
.where(Message.recipient_id == user.id, Message.read_at.is_(None))
|
|
||||||
)
|
|
||||||
manager.push(user.id, {"type": "unread", "count": int(n or 0)})
|
|
||||||
return {"partner": partner, "items": [_serialize_msg(m) for m in msgs]}
|
return {"partner": partner, "items": [_serialize_msg(m) for m in msgs]}
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -52,25 +52,32 @@ function UsersRoles({ me }: { me: Me }) {
|
||||||
const qc = useQueryClient();
|
const qc = useQueryClient();
|
||||||
const confirm = useConfirm();
|
const confirm = useConfirm();
|
||||||
const q = useQuery({ queryKey: ["admin-users"], queryFn: api.adminUsers });
|
const q = useQuery({ queryKey: ["admin-users"], queryFn: api.adminUsers });
|
||||||
// Which row currently has an action in flight, so we disable ONLY that row's button (mutations
|
// Which rows have an action in flight, so we disable ONLY those rows' buttons (SC1) — a Set, since
|
||||||
// are single-flight, so one id suffices) rather than greying every row (SC1).
|
// with per-row disabling the admin can now start actions on several rows concurrently.
|
||||||
const [pendingId, setPendingId] = useState<number | null>(null);
|
const [pendingIds, setPendingIds] = useState<Set<number>>(new Set());
|
||||||
|
const startPending = (id: number) => setPendingIds((s) => new Set(s).add(id));
|
||||||
|
const endPending = (id: number) =>
|
||||||
|
setPendingIds((s) => {
|
||||||
|
const n = new Set(s);
|
||||||
|
n.delete(id);
|
||||||
|
return n;
|
||||||
|
});
|
||||||
const setRole = useMutation({
|
const setRole = useMutation({
|
||||||
mutationFn: ({ id, role }: { id: number; role: "user" | "admin"; email: string }) =>
|
mutationFn: ({ id, role }: { id: number; role: "user" | "admin"; email: string }) =>
|
||||||
api.setUserRole(id, role),
|
api.setUserRole(id, role),
|
||||||
onMutate: (vars) => setPendingId(vars.id),
|
onMutate: (vars) => startPending(vars.id),
|
||||||
onSuccess: (_d, vars) => {
|
onSuccess: (_d, vars) => {
|
||||||
qc.invalidateQueries({ queryKey: ["admin-users"] });
|
qc.invalidateQueries({ queryKey: ["admin-users"] });
|
||||||
notify({ level: "success", message: t("users.roles.updated", { email: vars.email }) });
|
notify({ level: "success", message: t("users.roles.updated", { email: vars.email }) });
|
||||||
},
|
},
|
||||||
onSettled: () => setPendingId(null),
|
onSettled: (_d, _e, vars) => endPending(vars.id),
|
||||||
// Errors (e.g. last-admin / self) surface via the global error dialog with the server's detail.
|
// Errors (e.g. last-admin / self) surface via the global error dialog with the server's detail.
|
||||||
});
|
});
|
||||||
|
|
||||||
const suspend = useMutation({
|
const suspend = useMutation({
|
||||||
mutationFn: ({ id, suspended }: { id: number; suspended: boolean; email: string }) =>
|
mutationFn: ({ id, suspended }: { id: number; suspended: boolean; email: string }) =>
|
||||||
api.setUserSuspended(id, suspended),
|
api.setUserSuspended(id, suspended),
|
||||||
onMutate: (vars) => setPendingId(vars.id),
|
onMutate: (vars) => startPending(vars.id),
|
||||||
onSuccess: (_d, vars) => {
|
onSuccess: (_d, vars) => {
|
||||||
qc.invalidateQueries({ queryKey: ["admin-users"] });
|
qc.invalidateQueries({ queryKey: ["admin-users"] });
|
||||||
notify({
|
notify({
|
||||||
|
|
@ -80,17 +87,17 @@ function UsersRoles({ me }: { me: Me }) {
|
||||||
}),
|
}),
|
||||||
});
|
});
|
||||||
},
|
},
|
||||||
onSettled: () => setPendingId(null),
|
onSettled: (_d, _e, vars) => endPending(vars.id),
|
||||||
// Guards (demo / self / last admin) surface via the global error dialog.
|
// Guards (demo / self / last admin) surface via the global error dialog.
|
||||||
});
|
});
|
||||||
const del = useMutation({
|
const del = useMutation({
|
||||||
mutationFn: ({ id }: { id: number; email: string }) => api.adminDeleteUser(id),
|
mutationFn: ({ id }: { id: number; email: string }) => api.adminDeleteUser(id),
|
||||||
onMutate: (vars) => setPendingId(vars.id),
|
onMutate: (vars) => startPending(vars.id),
|
||||||
onSuccess: (_d, vars) => {
|
onSuccess: (_d, vars) => {
|
||||||
qc.invalidateQueries({ queryKey: ["admin-users"] });
|
qc.invalidateQueries({ queryKey: ["admin-users"] });
|
||||||
notify({ level: "success", message: t("users.delete.done", { email: vars.email }) });
|
notify({ level: "success", message: t("users.delete.done", { email: vars.email }) });
|
||||||
},
|
},
|
||||||
onSettled: () => setPendingId(null),
|
onSettled: (_d, _e, vars) => endPending(vars.id),
|
||||||
});
|
});
|
||||||
|
|
||||||
const onToggle = async (u: AdminUserRow) => {
|
const onToggle = async (u: AdminUserRow) => {
|
||||||
|
|
@ -171,7 +178,7 @@ function UsersRoles({ me }: { me: Me }) {
|
||||||
>
|
>
|
||||||
<button
|
<button
|
||||||
onClick={() => onToggle(u)}
|
onClick={() => onToggle(u)}
|
||||||
disabled={u.is_demo || self || pendingId === u.id}
|
disabled={u.is_demo || self || pendingIds.has(u.id)}
|
||||||
className="shrink-0 glass-card glass-hover inline-flex items-center gap-1 px-2.5 py-1.5 rounded-lg text-xs disabled:opacity-40 transition"
|
className="shrink-0 glass-card glass-hover inline-flex items-center gap-1 px-2.5 py-1.5 rounded-lg text-xs disabled:opacity-40 transition"
|
||||||
>
|
>
|
||||||
<Shield className="w-3.5 h-3.5" />
|
<Shield className="w-3.5 h-3.5" />
|
||||||
|
|
@ -191,7 +198,7 @@ function UsersRoles({ me }: { me: Me }) {
|
||||||
>
|
>
|
||||||
<button
|
<button
|
||||||
onClick={() => onSuspend(u)}
|
onClick={() => onSuspend(u)}
|
||||||
disabled={u.is_demo || self || pendingId === u.id}
|
disabled={u.is_demo || self || pendingIds.has(u.id)}
|
||||||
aria-label={u.is_suspended ? t("users.suspend.undoAction") : t("users.suspend.action")}
|
aria-label={u.is_suspended ? t("users.suspend.undoAction") : t("users.suspend.action")}
|
||||||
className={`shrink-0 p-1.5 rounded-lg border disabled:opacity-40 transition ${
|
className={`shrink-0 p-1.5 rounded-lg border disabled:opacity-40 transition ${
|
||||||
u.is_suspended
|
u.is_suspended
|
||||||
|
|
@ -205,7 +212,7 @@ function UsersRoles({ me }: { me: Me }) {
|
||||||
<Tooltip hint={u.is_demo || self ? t("users.delete.locked") : t("users.delete.action")}>
|
<Tooltip hint={u.is_demo || self ? t("users.delete.locked") : t("users.delete.action")}>
|
||||||
<button
|
<button
|
||||||
onClick={() => onDelete(u)}
|
onClick={() => onDelete(u)}
|
||||||
disabled={u.is_demo || self || pendingId === u.id}
|
disabled={u.is_demo || self || pendingIds.has(u.id)}
|
||||||
aria-label={t("users.delete.action")}
|
aria-label={t("users.delete.action")}
|
||||||
className="shrink-0 p-1.5 rounded-lg border border-border text-muted hover:text-red-400 disabled:opacity-40 transition"
|
className="shrink-0 p-1.5 rounded-lg border border-border text-muted hover:text-red-400 disabled:opacity-40 transition"
|
||||||
>
|
>
|
||||||
|
|
|
||||||
|
|
@ -68,8 +68,16 @@ export default function ChatThread({
|
||||||
// refresh the badges — but ONLY when the incoming-message count actually changed, not on every
|
// refresh the badges — but ONLY when the incoming-message count actually changed, not on every
|
||||||
// 20s poll (which would needlessly refetch /conversations + /unread_count each tick).
|
// 20s poll (which would needlessly refetch /conversations + /unread_count each tick).
|
||||||
const lastIncoming = useRef(-1);
|
const lastIncoming = useRef(-1);
|
||||||
|
const lastPartner = useRef(partnerId);
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!q.dataUpdatedAt) return;
|
if (!q.dataUpdatedAt) return;
|
||||||
|
// The Messages page reuses one ChatThread instance across conversation switches (not keyed by
|
||||||
|
// partnerId), so reset the counter when the partner changes — otherwise switching to a thread
|
||||||
|
// with the same incoming-count would skip the open-marks-read refresh.
|
||||||
|
if (lastPartner.current !== partnerId) {
|
||||||
|
lastPartner.current = partnerId;
|
||||||
|
lastIncoming.current = -1;
|
||||||
|
}
|
||||||
const incoming = (q.data?.items ?? []).filter((m) => m.recipient_id === meId).length;
|
const incoming = (q.data?.items ?? []).filter((m) => m.recipient_id === meId).length;
|
||||||
if (incoming !== lastIncoming.current) {
|
if (incoming !== lastIncoming.current) {
|
||||||
lastIncoming.current = incoming;
|
lastIncoming.current = incoming;
|
||||||
|
|
@ -77,7 +85,7 @@ export default function ChatThread({
|
||||||
qc.invalidateQueries({ queryKey: ["message-unread"] });
|
qc.invalidateQueries({ queryKey: ["message-unread"] });
|
||||||
}
|
}
|
||||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||||
}, [q.dataUpdatedAt, meId, qc]);
|
}, [q.dataUpdatedAt, partnerId, meId, qc]);
|
||||||
|
|
||||||
// Scroll to the newest message when the count changes (open / new message) — NOT on `plain`,
|
// Scroll to the newest message when the count changes (open / new message) — NOT on `plain`,
|
||||||
// which gets a fresh object on every poll's decrypt pass and would yank a scrolled-up reader
|
// which gets a fresh object on every poll's decrypt pass and would yank a scrolled-up reader
|
||||||
|
|
|
||||||
|
|
@ -299,6 +299,29 @@ const setupHeaders = (token: string) => ({
|
||||||
const ACTIVE_ACCOUNT_HEADER = "X-Siftlode-Account";
|
const ACTIVE_ACCOUNT_HEADER = "X-Siftlode-Account";
|
||||||
|
|
||||||
export const getActiveAccount = activeAccountId;
|
export const getActiveAccount = activeAccountId;
|
||||||
|
|
||||||
|
// Fire-and-forget POST that survives page unload (keepalive) — shared by the resume-position
|
||||||
|
// beacons (YT + Plex players). A normal fetch is cancelled on unload and React effect cleanup
|
||||||
|
// doesn't run on a full reload, so without keepalive the last position (esp. right after a seek)
|
||||||
|
// would be lost.
|
||||||
|
function beacon(url: string, body: Record<string, unknown>): void {
|
||||||
|
const active = getActiveAccount();
|
||||||
|
try {
|
||||||
|
void fetch(url, {
|
||||||
|
method: "POST",
|
||||||
|
keepalive: true,
|
||||||
|
credentials: "include",
|
||||||
|
headers: {
|
||||||
|
"Content-Type": "application/json",
|
||||||
|
...(active != null ? { [ACTIVE_ACCOUNT_HEADER]: String(active) } : {}),
|
||||||
|
},
|
||||||
|
body: JSON.stringify(body),
|
||||||
|
}).catch(() => {});
|
||||||
|
} catch {
|
||||||
|
/* ignore */
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export function setActiveAccount(id: number): void {
|
export function setActiveAccount(id: number): void {
|
||||||
try {
|
try {
|
||||||
sessionStorage.setItem(ACTIVE_ACCOUNT_KEY, String(id));
|
sessionStorage.setItem(ACTIVE_ACCOUNT_KEY, String(id));
|
||||||
|
|
@ -1102,30 +1125,12 @@ export const api = {
|
||||||
},
|
},
|
||||||
{ idempotent: true }
|
{ idempotent: true }
|
||||||
),
|
),
|
||||||
// Fire-and-forget YT progress save that survives page unload (F5/close/navigate) — mirrors
|
// Fire-and-forget YT progress save that survives page unload (F5/close/navigate).
|
||||||
// plexProgressBeacon. A normal fetch is cancelled on unload and React effect cleanup doesn't run
|
saveProgressBeacon: (id: string, positionSeconds: number, durationSeconds: number): void =>
|
||||||
// on a full reload, so the last position (esp. right after a seek) would be lost; keepalive lets
|
beacon(`/api/videos/${encodeURIComponent(id)}/progress`, {
|
||||||
// it complete.
|
position_seconds: Math.floor(positionSeconds),
|
||||||
saveProgressBeacon: (id: string, positionSeconds: number, durationSeconds: number): void => {
|
duration_seconds: Math.floor(durationSeconds),
|
||||||
const active = getActiveAccount();
|
}),
|
||||||
try {
|
|
||||||
void fetch(`/api/videos/${encodeURIComponent(id)}/progress`, {
|
|
||||||
method: "POST",
|
|
||||||
keepalive: true,
|
|
||||||
credentials: "include",
|
|
||||||
headers: {
|
|
||||||
"Content-Type": "application/json",
|
|
||||||
...(active != null ? { [ACTIVE_ACCOUNT_HEADER]: String(active) } : {}),
|
|
||||||
},
|
|
||||||
body: JSON.stringify({
|
|
||||||
position_seconds: Math.floor(positionSeconds),
|
|
||||||
duration_seconds: Math.floor(durationSeconds),
|
|
||||||
}),
|
|
||||||
}).catch(() => {});
|
|
||||||
} catch {
|
|
||||||
/* ignore */
|
|
||||||
}
|
|
||||||
},
|
|
||||||
videoDetail: (id: string): Promise<VideoDetail> => req(`/api/videos/${id}`),
|
videoDetail: (id: string): Promise<VideoDetail> => req(`/api/videos/${id}`),
|
||||||
pauseSync: () => req("/api/sync/pause", { method: "POST" }),
|
pauseSync: () => req("/api/sync/pause", { method: "POST" }),
|
||||||
resumeSync: () => req("/api/sync/resume", { method: "POST" }),
|
resumeSync: () => req("/api/sync/resume", { method: "POST" }),
|
||||||
|
|
@ -1358,23 +1363,12 @@ export const api = {
|
||||||
position_seconds: number,
|
position_seconds: number,
|
||||||
duration_seconds: number,
|
duration_seconds: number,
|
||||||
final = false,
|
final = false,
|
||||||
): void => {
|
): void =>
|
||||||
const active = getActiveAccount();
|
beacon(`/api/plex/item/${encodeURIComponent(id)}/progress`, {
|
||||||
try {
|
position_seconds,
|
||||||
void fetch(`/api/plex/item/${encodeURIComponent(id)}/progress`, {
|
duration_seconds,
|
||||||
method: "POST",
|
final,
|
||||||
keepalive: true,
|
}),
|
||||||
credentials: "include",
|
|
||||||
headers: {
|
|
||||||
"Content-Type": "application/json",
|
|
||||||
...(active != null ? { [ACTIVE_ACCOUNT_HEADER]: String(active) } : {}),
|
|
||||||
},
|
|
||||||
body: JSON.stringify({ position_seconds, duration_seconds, final }),
|
|
||||||
}).catch(() => {});
|
|
||||||
} catch {
|
|
||||||
/* ignore */
|
|
||||||
}
|
|
||||||
},
|
|
||||||
plexSetState: (id: string, status: "new" | "watched" | "hidden"): Promise<unknown> =>
|
plexSetState: (id: string, status: "new" | "watched" | "hidden"): Promise<unknown> =>
|
||||||
req(`/api/plex/item/${encodeURIComponent(id)}/state`, {
|
req(`/api/plex/item/${encodeURIComponent(id)}/state`, {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue