refactor(review): address #230 third review — callout dedup, ticket/type tidy
Approve-with-comments follow-ups (no blockers): - callout: unify the GitHub-callout feature ticket on #192 (the callout-paste feature the CHANGELOG already tracks); #218 is the public-share security work. Fixed the code comment and test reference. - export/utils.spec: pin current behavior of a leading-dot name (".gitignore" -> "") — same bug class as #204 but unreachable via the sole caller, so document not change. - share.types: narrow ISharedPage to the actual /shares/page-info allowlist (page -> Pick of id/slugId/title/icon/content; trimmed share; dropped the spurious `extends IShare`). Verified all three consumers (shared-page, link-view, mention-view) read only allowlist fields. - editor-ext: extract shared CALLOUT_TYPES / normalizeCalloutType / renderCalloutHtml into callout-common.marked.ts; both tokenizers (`:::type` and `> [!type]`) now share the renderer + type dict while staying separate. Eliminates the byte-identical renderer + duplicated type list. - share.service: extract named predicate shareIdGrantsAccess(requestedShareId, resolvedShare) for the id-or-key fast path (naming only, no control-flow change); kept narrower than resolveReadableSharePage's id-only gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -253,10 +253,7 @@ export class ShareService {
|
||||
workspaceId: string,
|
||||
): Promise<boolean> {
|
||||
// Fast path: the request names the page's own resolved share.
|
||||
if (
|
||||
requestedShareId === resolvedShare.id ||
|
||||
requestedShareId.toLowerCase() === resolvedShare.key?.toLowerCase()
|
||||
) {
|
||||
if (this.shareIdGrantsAccess(requestedShareId, resolvedShare)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -270,6 +267,23 @@ export class ShareService {
|
||||
return !!ancestor;
|
||||
}
|
||||
|
||||
/**
|
||||
* Does the requested share id/key directly name `resolvedShare` — by id, or
|
||||
* by key (case-insensitive)? This is the "names the page's OWN share" half of
|
||||
* the access concept; ancestor includeSubPages shares are matched separately.
|
||||
* Intentionally narrower than `resolveReadableSharePage`'s id-only gate, which
|
||||
* keeps its own contract for the callers that pass a shareId there.
|
||||
*/
|
||||
private shareIdGrantsAccess(
|
||||
requestedShareId: string,
|
||||
resolvedShare: { id: string; key?: string | null },
|
||||
): boolean {
|
||||
return (
|
||||
requestedShareId === resolvedShare.id ||
|
||||
requestedShareId.toLowerCase() === resolvedShare.key?.toLowerCase()
|
||||
);
|
||||
}
|
||||
|
||||
async getShareForPage(pageId: string, workspaceId: string) {
|
||||
// here we try to check if a page was shared directly or if it inherits the share from its closest shared ancestor
|
||||
const share = await this.db
|
||||
|
||||
@@ -159,6 +159,14 @@ describe('getInternalLinkPageName', () => {
|
||||
expect(getInternalLinkPageName('docs/v1.2.md')).toBe('v1.2');
|
||||
});
|
||||
|
||||
it('documents current behavior: a leading-dot name collapses to empty text', () => {
|
||||
// ".gitignore" -> base ".gitignore", parts ["", "gitignore"]: the leading
|
||||
// dot is treated as a (empty) name + extension, so the name drops to "".
|
||||
// Same bug class as #204, but unreachable via the sole caller (page titles
|
||||
// never start with a dot), so we only pin the behavior — not fix it.
|
||||
expect(getInternalLinkPageName('.gitignore')).toBe('');
|
||||
});
|
||||
|
||||
it('falls back to the raw name without throwing on malformed encoding', () => {
|
||||
// "%E0%A4" is an incomplete escape; decodeURIComponent throws and the
|
||||
// helper returns the raw (still-encoded) name.
|
||||
|
||||
Reference in New Issue
Block a user