diff --git a/src/identity/providers.ts b/src/identity/providers.ts index 4976fc5..3c88baf 100644 --- a/src/identity/providers.ts +++ b/src/identity/providers.ts @@ -68,6 +68,33 @@ export class ConfiguredIdentityResolver implements IdentityResolver { } } +/** + * Fill a resolved identity's missing appearance from the configured directory. + * + * Who an ID belongs to and what they look like come from different places. A live provider knows + * the first; whether it knows the second depends on the account it signed in with - Relay carries + * a picture only when the OAuth provider supplied one, and a colour only for the local user. The + * configured directory is where someone records what a provider cannot tell them. + * + * So a provider wins on identity while the directory fills the gaps, rather than the first + * resolver with any answer at all taking the whole record. Otherwise directory entries are ignored + * for exactly the people a provider already recognises, which is normally all of them - and the + * setting appears to do nothing. + */ +export function withConfiguredDecoration( + identity: Identity, + configured: Identity | null, +): Identity { + if (!configured) return identity; + + return { + ...identity, + picture: identity.picture ?? configured.picture, + color: identity.color ?? configured.color, + colorLight: identity.colorLight ?? configured.colorLight, + }; +} + export type RelayIdentitySupportStatus = | "not-installed" | "unsupported" diff --git a/src/main.ts b/src/main.ts index 83c817d..d235432 100644 --- a/src/main.ts +++ b/src/main.ts @@ -72,6 +72,7 @@ import { getRelayIdentitySupportStatus, providerById, selectIdentityProvider, + withConfiguredDecoration, } from "./identity/providers"; import type { Identity, @@ -1353,7 +1354,8 @@ export default class RelayCommentsPlugin if (!resolver.isAvailable()) continue; try { const identity = await resolver.resolveUser(author, path); - if (identity) return providerIdentity(identity, resolver.id); + if (identity) + return this.decorated(identity, resolver, author, path); } catch { // Resolver failures degrade to the unresolved author value // instead of breaking review rendering. @@ -1364,6 +1366,47 @@ export default class RelayCommentsPlugin return null; } + /** + * One resolver's answer, with appearance the directory carries and it does not. + * + * Skipped when the directory is itself the answering resolver, which has nothing to fill in + * from. + */ + private decorated( + identity: Identity, + resolver: IdentityResolver, + author: string, + path: string, + ): ReviewerIdentity { + if (resolver.id === this.configuredIdentityResolver.id) + return providerIdentity(identity, resolver.id); + + return providerIdentity( + withConfiguredDecoration( + identity, + this.resolveConfiguredIdentity(author, path), + ), + resolver.id, + ); + } + + /** The configured directory's record for an author, for decoration only. */ + private resolveConfiguredIdentity( + author: string, + path: string, + ): Identity | null { + if (!this.configuredIdentityResolver.isAvailable()) return null; + + try { + return ( + this.configuredIdentityResolver.resolveUserSnapshot?.(author, path) ?? + null + ); + } catch { + return null; + } + } + private resolveAuthorIdentitySnapshot( author: string, path: string, @@ -1378,7 +1421,8 @@ export default class RelayCommentsPlugin if (!resolver.resolveUserSnapshot) return undefined; try { const identity = resolver.resolveUserSnapshot(author, path); - if (identity) return providerIdentity(identity, resolver.id); + if (identity) + return this.decorated(identity, resolver, author, path); } catch { return undefined; } diff --git a/tests/unit/identity/providers.test.ts b/tests/unit/identity/providers.test.ts index c3bd5b5..85b6f96 100644 --- a/tests/unit/identity/providers.test.ts +++ b/tests/unit/identity/providers.test.ts @@ -7,8 +7,69 @@ import { ObsidianSyncIdentityProvider, RelayIdentityProvider, selectIdentityProvider, + withConfiguredDecoration, } from "src/identity/providers"; +describe("configured decoration", () => { + it("supplies the appearance a provider does not carry", () => { + expect( + withConfiguredDecoration( + { id: "u1", name: "Sarah Rilling" }, + { + id: "u1", + name: "whatever the directory calls them", + picture: "https://example.com/sarah.jpg", + color: "#30bced", + colorLight: "#30bced33", + }, + ), + ).toEqual({ + id: "u1", + // The provider stays authoritative about who this is. + name: "Sarah Rilling", + picture: "https://example.com/sarah.jpg", + color: "#30bced", + colorLight: "#30bced33", + }); + }); + + it("never overrides appearance a provider does carry", () => { + expect( + withConfiguredDecoration( + { id: "u1", name: "Sarah", picture: "provider.jpg", color: "#111111" }, + { + id: "u1", + name: "Sarah", + picture: "configured.jpg", + color: "#222222", + }, + ), + ).toMatchObject({ picture: "provider.jpg", color: "#111111" }); + }); + + it("leaves an identity alone when the directory has no entry", () => { + const identity = { id: "u1", name: "Sarah" }; + + expect(withConfiguredDecoration(identity, null)).toEqual(identity); + }); + + it("fills each field independently", () => { + // A directory entry that only sets a colour must not blank out a picture, and vice versa. + expect( + withConfiguredDecoration( + { id: "u1", name: "Sarah", picture: "provider.jpg" }, + { id: "u1", name: "Sarah", color: "#30bced" }, + ), + ).toEqual({ + id: "u1", + name: "Sarah", + picture: "provider.jpg", + color: "#30bced", + colorLight: undefined, + }); + }); +}); + describe("configured identity directory", () => { it("resolves other authors without acting as the current user", async () => { const provider = new ConfiguredIdentityResolver(() => [