Compare commits

...
Author SHA1 Message Date
Nimrod Gutman ba699a114a fix(skill-detail): stop owner canonical redirect loop 2026-03-28 20:43:55 +03:00
5 changed files with 142 additions and 12 deletions
+13 -1
View File
@@ -37,6 +37,7 @@ function makeCtx() {
slug: "padel",
displayName: "Padel",
ownerUserId: "users:owner",
ownerPublisherId: "publishers:local",
latestVersionId: "skillVersions:1",
manualOverride: {
verdict: "clean",
@@ -103,6 +104,15 @@ function makeCtx() {
switch (id) {
case "skillVersions:1":
return latestVersion;
case "publishers:local":
return {
_id: "publishers:local",
_creationTime: 1,
kind: "user",
handle: "local-publisher",
displayName: "Local Dev",
linkedUserId: "users:owner",
};
case "users:owner":
return {
_id: "users:owner",
@@ -150,7 +160,7 @@ describe("getBySlugForStaff audit logs", () => {
vi.mocked(requireUser).mockReset();
});
it("returns reviewer info and recent audit logs with actor handles", async () => {
it("returns publisher-backed owner info plus recent audit logs with actor handles", async () => {
vi.mocked(requireUser).mockResolvedValue({
userId: "users:moderator",
user: { _id: "users:moderator", role: "moderator" },
@@ -162,6 +172,7 @@ describe("getBySlugForStaff audit logs", () => {
slug: "padel",
auditLogLimit: 5,
})) as {
owner: { handle?: string | null } | null;
overrideReviewer: { handle?: string | null } | null;
auditLogs: Array<{
actor: { handle?: string | null } | null;
@@ -171,6 +182,7 @@ describe("getBySlugForStaff audit logs", () => {
expect(getSkillBadgeMap).toHaveBeenCalled();
expect(auditTake).toHaveBeenCalledWith(5);
expect(result.owner?.handle).toBe("local-publisher");
expect(result.overrideReviewer?.handle).toBe("moddy");
expect(result.auditLogs).toHaveLength(2);
expect(result.auditLogs[0]?.action).toBe("skill.manual_override.set");
+21 -7
View File
@@ -1632,7 +1632,11 @@ export const getBySlugForStaff = query({
if (!skill) return null;
const latestVersion = skill.latestVersionId ? await ctx.db.get(skill.latestVersionId) : null;
const owner = toPublicUser(await ctx.db.get(skill.ownerUserId));
const ownerPublisher = await getOwnerPublisher(ctx, {
ownerPublisherId: skill.ownerPublisherId,
ownerUserId: skill.ownerUserId,
});
const owner = toPublicPublisher(ownerPublisher);
const badges = await getSkillBadgeMap(ctx, skill._id);
const rawAuditLogs = await ctx.db
.query("auditLogs")
@@ -1659,10 +1663,20 @@ export const getBySlugForStaff = query({
}));
const forkOfSkill = skill.forkOf?.skillId ? await ctx.db.get(skill.forkOf.skillId) : null;
const forkOfOwner = forkOfSkill ? await ctx.db.get(forkOfSkill.ownerUserId) : null;
const forkOfOwner = forkOfSkill
? await getOwnerPublisher(ctx, {
ownerPublisherId: forkOfSkill.ownerPublisherId,
ownerUserId: forkOfSkill.ownerUserId,
})
: null;
const canonicalSkill = skill.canonicalSkillId ? await ctx.db.get(skill.canonicalSkillId) : null;
const canonicalOwner = canonicalSkill ? await ctx.db.get(canonicalSkill.ownerUserId) : null;
const canonicalOwner = canonicalSkill
? await getOwnerPublisher(ctx, {
ownerPublisherId: canonicalSkill.ownerPublisherId,
ownerUserId: canonicalSkill.ownerUserId,
})
: null;
return {
requestedSlug: resolved.requestedSlug,
@@ -1681,8 +1695,8 @@ export const getBySlugForStaff = query({
displayName: forkOfSkill.displayName,
},
owner: {
handle: forkOfOwner?.handle ?? forkOfOwner?.name ?? null,
userId: forkOfOwner?._id ?? null,
handle: forkOfOwner?.handle ?? null,
userId: forkOfOwner?.linkedUserId ?? null,
},
}
: null,
@@ -1693,8 +1707,8 @@ export const getBySlugForStaff = query({
displayName: canonicalSkill.displayName,
},
owner: {
handle: canonicalOwner?.handle ?? canonicalOwner?.name ?? null,
userId: canonicalOwner?._id ?? null,
handle: canonicalOwner?.handle ?? null,
userId: canonicalOwner?.linkedUserId ?? null,
},
}
: null,
+99
View File
@@ -7,6 +7,7 @@ const navigateMock = vi.fn();
const useAuthStatusMock = vi.fn();
vi.mock("@tanstack/react-router", () => ({
Link: ({ children }: { children: unknown }) => children,
useNavigate: () => navigateMock,
}));
@@ -258,6 +259,104 @@ describe("SkillDetailPage", () => {
});
});
it("does not redirect when a staff owner handle only differs by case", async () => {
useAuthStatusMock.mockReturnValue({
isAuthenticated: true,
isLoading: false,
me: { _id: "users:staff", role: "moderator" },
});
useQueryMock.mockImplementation((_fn: unknown, args: unknown) => {
if (args === "skip") return undefined;
if (args && typeof args === "object" && "skillId" in args) return [];
if (args && typeof args === "object" && "slug" in args) {
return {
skill: {
_id: "skills:1",
slug: "weather",
displayName: "Weather",
summary: "Get current weather.",
ownerUserId: "users:1",
ownerPublisherId: "publishers:steipete",
tags: {},
stats: { stars: 0, downloads: 0 },
},
owner: {
_id: "publishers:steipete",
_creationTime: 0,
kind: "user",
handle: "SteiPete",
displayName: "Peter",
linkedUserId: "users:1",
},
latestVersion: { _id: "skillVersions:1", version: "1.0.0", parsed: {}, files: [] },
forkOf: null,
canonical: null,
};
}
return undefined;
});
render(
<SkillDetailPage
slug="weather"
canonicalOwner="steipete"
initialData={{
result: {
skill: {
_id: skillId,
_creationTime: 0,
slug: "weather",
displayName: "Weather",
summary: "Get current weather.",
ownerUserId: ownerId,
ownerPublisherId,
tags: {},
badges: {},
stats: {
stars: 12,
downloads: 34,
installsCurrent: 5,
installsAllTime: 8,
versions: 1,
comments: 0,
},
createdAt: 0,
updatedAt: 0,
},
owner: {
_id: ownerPublisherId,
_creationTime: 0,
kind: "user",
handle: "steipete",
displayName: "Peter",
linkedUserId: ownerId,
},
latestVersion: {
_id: versionId,
_creationTime: 0,
skillId,
version: "1.0.0",
fingerprint: "abc",
changelog: "Initial release",
parsed: { license: "MIT-0", frontmatter: {} },
files: [],
createdBy: ownerId,
createdAt: 0,
},
forkOf: null,
canonical: null,
},
readme: "# Weather",
readmeError: null,
}}
/>,
);
expect(screen.queryByText(/Loading skill/i)).toBeNull();
expect(screen.getAllByText("Weather").length).toBeGreaterThan(0);
expect(navigateMock).not.toHaveBeenCalled();
});
it("opens report dialog for authenticated users", async () => {
useAuthStatusMock.mockReturnValue({
isAuthenticated: true,
+4 -2
View File
@@ -144,12 +144,14 @@ export function SkillDetailPage({
) as Array<{ _id: Id<"skills">; slug: string; displayName: string }> | undefined;
const ownerHandle = owner?.handle ?? null;
const ownerParam = ownerHandle ?? (owner?._id ? String(owner._id) : null);
const ownerParam = ownerHandle?.trim().toLowerCase() || (owner?._id ? String(owner._id) : null);
const canonicalOwnerParam =
typeof canonicalOwner === "string" ? canonicalOwner.trim().toLowerCase() : null;
const wantsCanonicalRedirect = Boolean(
ownerParam &&
((result?.resolvedSlug && result.resolvedSlug !== slug) ||
redirectToCanonical ||
(typeof canonicalOwner === "string" && canonicalOwner && canonicalOwner !== ownerParam)),
(canonicalOwnerParam && canonicalOwnerParam !== ownerParam)),
);
const forkOf = result?.forkOf ?? null;
+5 -2
View File
@@ -72,8 +72,11 @@ type SkillBySlugResult = {
} | null;
} | null;
function resolveOwnerParam(handle: string | null | undefined, ownerId?: Id<"users">) {
return handle?.trim() || (ownerId ? String(ownerId) : "unknown");
function resolveOwnerParam(
handle: string | null | undefined,
ownerId?: Id<"users"> | Id<"publishers">,
) {
return handle?.trim().toLowerCase() || (ownerId ? String(ownerId) : "unknown");
}
function promptBanReason(label: string) {