From 4861990551ea6bb3404fc0f93ab70dadc5682f9a Mon Sep 17 00:00:00 2001 From: sdarbinyan Date: Fri, 17 Jul 2026 09:43:00 +0400 Subject: [PATCH] feat(static-pages): extend data model with route/enabled/status/media/SEO Milestone 1 of the Static Pages Module sprint. - StaticPageConfig / ContentPage / ContentPageBootstrapInput gain: explicit editable route (defaults from slug, independently overridable), enabled (master on/off), status: 'draft'|'published' (per-page publish lifecycle, independent of the whole-bootstrap draft/publish cycle), customTemplate, heroImage/thumbnail/gallery, and seo.robots. - ContentPageService: normalizePage/normalizePages default missing enabled/status to enabled+published so existing bootstrap data never gets silently un-published; only the editor's createPage() opts a brand-new page into 'draft'. Legacy array-format pages get the same treatment. - resolvePage now returns null (storefront 404) for a disabled or draft page, regardless of whether the surrounding bootstrap itself is published - affects the storefront static-page route AND the auto-generated footer nav group (both go through this same resolver), which is the correct behavior. - validatePages extended: duplicateRoutes (route can now diverge from slug), invalidHtml, invalidSeo (canonical/ogImage URL shape, known robots tokens). - New schema/validators/primitives.validateHtml: stack-based tag-balance check (void/self-closing elements skipped, comments stripped). Caught and fixed a real bug during its own spec run: the initial implementation popped the stack back to the nearest matching ancestor on a mismatched closing tag, which silently swallowed a genuinely unclosed inner tag instead of flagging it - now a closing tag must match the top of the stack exactly. - toBootstrapRecord serializes the new fields; visible mirrors enabled so any reader of the older field name stays truthful. - Specs: content-page.service.spec.ts (new), primitives.spec.ts (validateHtml). Gate: tsc --noEmit, npm test (57/57), arch:check, build all green. Co-Authored-By: Claude Opus 4.8 --- .../static-pages-editor.component.ts | 9 +- .../models/content-page.model.ts | 20 +++ .../services/content-page.service.spec.ts | 158 ++++++++++++++++++ .../services/content-page.service.ts | 131 ++++++++++++--- .../schema/validators/primitives.spec.ts | 30 ++++ .../schema/validators/primitives.ts | 52 ++++++ .../shared/models/config/static-page.model.ts | 13 ++ 7 files changed, 388 insertions(+), 25 deletions(-) create mode 100644 src/app/features/content-management/services/content-page.service.spec.ts diff --git a/src/app/features/content-management/components/static-pages-editor.component.ts b/src/app/features/content-management/components/static-pages-editor.component.ts index 11e5c70..59dec1b 100644 --- a/src/app/features/content-management/components/static-pages-editor.component.ts +++ b/src/app/features/content-management/components/static-pages-editor.component.ts @@ -40,9 +40,11 @@ export class StaticPagesEditorComponent { readonly locales = computed(() => this.bootstrap()?.localization.supportedLocales ?? ['en']); createPage(): void { + const slug = `custom-page-${this.pages().length + 1}`; const page: ContentPage = { id: `page-${Date.now()}`, - slug: `custom-page-${this.pages().length + 1}`, + slug, + route: slug, title: '', order: this.pages().length + 1, showInFooter: false, @@ -55,6 +57,11 @@ export class StaticPagesEditorComponent { ru: { title: '', html: '' }, hy: { title: '', html: '' }, }, + // New pages start disabled from the storefront's perspective until an + // author explicitly publishes them - existing bootstrap pages default + // to enabled/published on normalize (see ContentPageService.normalizePage). + enabled: true, + status: 'draft', }; this.persist([...this.pages(), page]); diff --git a/src/app/features/content-management/models/content-page.model.ts b/src/app/features/content-management/models/content-page.model.ts index 9530ae1..4f6d7e6 100644 --- a/src/app/features/content-management/models/content-page.model.ts +++ b/src/app/features/content-management/models/content-page.model.ts @@ -8,6 +8,7 @@ export interface ContentPageSeoConfig { ogTitle?: string; ogDescription?: string; ogImage?: string; + robots?: string; } export interface ContentPageTranslation { @@ -16,9 +17,13 @@ export interface ContentPageTranslation { seo?: ContentPageSeoConfig; } +export type ContentPageStatus = 'draft' | 'published'; + export interface ContentPage { id: string; slug: string; + /** Editable independently of `slug`; defaults from it but may diverge (e.g. legacy redirects). */ + route: string; title: string; icon?: string; order: number; @@ -34,11 +39,20 @@ export interface ContentPage { footerGroup?: string; translations: Record; seo?: ContentPageSeoConfig; + /** Master on/off switch; a disabled page never resolves on the storefront. */ + enabled: boolean; + /** Per-page publish lifecycle, independent of the whole-bootstrap draft/publish cycle. */ + status: ContentPageStatus; + customTemplate?: string; + heroImage?: string; + thumbnail?: string; + gallery?: string[]; } export interface ContentPageBootstrapInput { id: string; slug: string; + route?: string; title?: string | LocalizedTextContent; showInFooter?: boolean; showInHeader?: boolean; @@ -55,4 +69,10 @@ export interface ContentPageBootstrapInput { translations?: Record; html?: string | LocalizedHtmlContent; seo?: ContentPageSeoConfig | { title?: string | LocalizedTextContent; description?: string | LocalizedTextContent; keywords?: string; canonical?: string; ogTitle?: string | LocalizedTextContent; ogDescription?: string | LocalizedTextContent; ogImage?: string; robots?: string; metaTags?: Array<{ name?: string; property?: string; content: string }>; }; + enabled?: boolean; + status?: ContentPageStatus; + customTemplate?: string; + heroImage?: string; + thumbnail?: string; + gallery?: string[]; } diff --git a/src/app/features/content-management/services/content-page.service.spec.ts b/src/app/features/content-management/services/content-page.service.spec.ts new file mode 100644 index 0000000..ff4aca5 --- /dev/null +++ b/src/app/features/content-management/services/content-page.service.spec.ts @@ -0,0 +1,158 @@ +import { BootstrapConfig, StaticPageConfig } from '../../../shared/models/config'; +import { ContentPage } from '../models/content-page.model'; +import { ContentPageService } from './content-page.service'; + +function makePage(overrides: Partial = {}): StaticPageConfig { + return { + id: 'about', + slug: 'about', + title: 'About', + translations: { en: { title: 'About', html: '

Hello

' } }, + ...overrides, + }; +} + +function makeContentPage(overrides: Partial = {}): ContentPage { + return { + id: 'about', + slug: 'about', + route: 'about', + title: 'About', + order: 1, + showInFooter: true, + showInHeader: false, + showInSitemap: true, + visibility: { desktop: true, tablet: true, mobile: true }, + requiresAuthentication: false, + translations: { en: { title: 'About', html: '

Hello

' } }, + enabled: true, + status: 'published', + ...overrides, + }; +} + +describe('ContentPageService', () => { + let service: ContentPageService; + + beforeEach(() => { + service = new ContentPageService(); + }); + + describe('normalizePages', () => { + it('defaults a page with no enabled/status to enabled + published (preserves pre-existing bootstrap data)', () => { + const [page] = service.normalizePages({ about: makePage() }); + expect(page.enabled).toBeTrue(); + expect(page.status).toBe('published'); + }); + + it('respects explicit enabled/status when present', () => { + const [page] = service.normalizePages({ about: makePage({ enabled: false, status: 'draft' }) }); + expect(page.enabled).toBeFalse(); + expect(page.status).toBe('draft'); + }); + + it('defaults route from slug when route is absent', () => { + const [page] = service.normalizePages({ about: makePage({ slug: 'about-us' }) }); + expect(page.route).toBe('about-us'); + }); + + it('keeps an explicitly different route independent of slug', () => { + const [page] = service.normalizePages({ about: makePage({ slug: 'about-us', route: '/legacy/about' }) }); + expect(page.route).toBe('legacy/about'); + }); + + it('normalizes legacy array-format pages as enabled + published', () => { + const [page] = service.normalizePages([{ key: 'terms', route: '/terms', title: 'Terms' }]); + expect(page.enabled).toBeTrue(); + expect(page.status).toBe('published'); + }); + }); + + describe('resolvePage', () => { + function bootstrapWith(page: StaticPageConfig): BootstrapConfig { + return { staticPages: { [page.id]: page } } as unknown as BootstrapConfig; + } + + it('resolves an enabled, published page', () => { + const resolved = service.resolvePage(bootstrapWith(makePage()), 'about', 'en'); + expect(resolved).not.toBeNull(); + expect(resolved?.html).toBe('

Hello

'); + }); + + it('returns null for a disabled page even though it exists', () => { + const resolved = service.resolvePage(bootstrapWith(makePage({ enabled: false })), 'about', 'en'); + expect(resolved).toBeNull(); + }); + + it('returns null for a draft page', () => { + const resolved = service.resolvePage(bootstrapWith(makePage({ status: 'draft' })), 'about', 'en'); + expect(resolved).toBeNull(); + }); + + it('returns null for a page that does not exist', () => { + const resolved = service.resolvePage(bootstrapWith(makePage()), 'missing', 'en'); + expect(resolved).toBeNull(); + }); + }); + + describe('validatePages', () => { + it('reports no issues for two distinct, valid pages', () => { + const pages = [makeContentPage({ id: 'a', slug: 'a', route: 'a' }), makeContentPage({ id: 'b', slug: 'b', route: 'b' })]; + const result = service.validatePages(pages); + expect(result.duplicateSlugs).toEqual([]); + expect(result.duplicateRoutes).toEqual([]); + expect(result.emptyTitles).toEqual([]); + expect(result.invalidHtml).toEqual([]); + expect(result.invalidSeo).toEqual([]); + }); + + it('flags duplicate routes even when slugs differ', () => { + const pages = [ + makeContentPage({ id: 'a', slug: 'a-page', route: 'shared' }), + makeContentPage({ id: 'b', slug: 'b-page', route: 'shared' }), + ]; + expect(service.validatePages(pages).duplicateRoutes).toEqual(['shared']); + }); + + it('flags malformed HTML content', () => { + const pages = [makeContentPage({ translations: { en: { title: 'About', html: '

Hello world

' } } })]; + expect(service.validatePages(pages).invalidHtml).toEqual(['about']); + }); + + it('flags an invalid canonical URL and an unknown robots token', () => { + const pages = [makeContentPage({ seo: { canonical: 'not-a-url', robots: 'maybe' } })]; + expect(service.validatePages(pages).invalidSeo).toEqual(['about']); + }); + + it('accepts a known robots token and a valid canonical URL', () => { + const pages = [makeContentPage({ seo: { canonical: 'https://dexar.market/about', robots: 'index,follow' } })]; + expect(service.validatePages(pages).invalidSeo).toEqual([]); + }); + }); + + describe('toBootstrapRecord', () => { + it('round-trips enabled/status/route/media through serialization', () => { + const page = makeContentPage({ enabled: false, status: 'draft', heroImage: 'hero.png', thumbnail: 'thumb.png', gallery: ['a.png'] }); + const record = service.toBootstrapRecord([page]); + expect(record['about'].enabled).toBeFalse(); + expect(record['about'].status).toBe('draft'); + expect(record['about'].visible).toBeFalse(); + expect(record['about'].heroImage).toBe('hero.png'); + expect(record['about'].thumbnail).toBe('thumb.png'); + expect(record['about'].gallery).toEqual(['a.png']); + expect(record['about'].route).toBe('/about'); + }); + + it('serializes an explicitly different route', () => { + const page = makeContentPage({ route: 'legacy/about' }); + const record = service.toBootstrapRecord([page]); + expect(record['about'].route).toBe('/legacy/about'); + }); + }); + + describe('normalizeSlug', () => { + it('strips leading slashes and trims', () => { + expect(service.normalizeSlug('/about ')).toBe('about'); + }); + }); +}); diff --git a/src/app/features/content-management/services/content-page.service.ts b/src/app/features/content-management/services/content-page.service.ts index 1b8d1ed..ca73f89 100644 --- a/src/app/features/content-management/services/content-page.service.ts +++ b/src/app/features/content-management/services/content-page.service.ts @@ -1,6 +1,9 @@ import { Injectable } from '@angular/core'; import { BootstrapConfig, LocalizedHtmlContent, LocalizedTextContent, ResolvedStaticPage, StaticPageConfig, StaticPagesConfig } from '../../../shared/models/config'; import { ContentPage, ContentPageBootstrapInput } from '../models/content-page.model'; +import { isValidHttpUrl, normalizeRoute, validateHtml } from '../../project-editor/schema/validators/primitives'; + +const KNOWN_ROBOTS_TOKENS = new Set(['index', 'noindex', 'follow', 'nofollow', 'index,follow', 'index,nofollow', 'noindex,follow', 'noindex,nofollow']); @Injectable({ providedIn: 'root' }) export class ContentPageService { @@ -10,21 +13,29 @@ export class ContentPageService { } if (Array.isArray(config)) { - return config.map((page, index) => ({ - id: page.key, - slug: this.normalizeSlug(typeof page.route === 'string' ? page.route : page.route?.path ?? page.key), - title: typeof page.title === 'string' ? page.title : page.title?.['en'] ?? page.key, - icon: undefined, - order: index + 1, - showInFooter: false, - showInHeader: false, - showInSitemap: true, - visibility: { desktop: true, tablet: true, mobile: true }, - requiresAuthentication: false, - footerGroup: undefined, - translations: this.normalizeLegacyTranslations(page.title, page.content), - seo: undefined, - })); + return config.map((page, index) => { + const slug = this.normalizeSlug(typeof page.route === 'string' ? page.route : page.route?.path ?? page.key); + return { + id: page.key, + slug, + route: slug, + title: typeof page.title === 'string' ? page.title : page.title?.['en'] ?? page.key, + icon: undefined, + order: index + 1, + showInFooter: false, + showInHeader: false, + showInSitemap: true, + visibility: { desktop: true, tablet: true, mobile: true }, + requiresAuthentication: false, + footerGroup: undefined, + translations: this.normalizeLegacyTranslations(page.title, page.content), + seo: undefined, + // Legacy array-format pages predate enabled/status; they were always live, so + // normalizing must keep them live rather than silently un-publishing them. + enabled: page.visible !== false, + status: 'published' as const, + }; + }); } return Object.entries(config) @@ -35,7 +46,9 @@ export class ContentPageService { resolvePage(bootstrap: BootstrapConfig, keyOrSlug: string, locale: string): ResolvedStaticPage | null { const normalizedPages = this.normalizePages(bootstrap.staticPages); const target = normalizedPages.find(page => page.id === keyOrSlug || page.slug === this.normalizeSlug(keyOrSlug)); - if (!target) { + // A disabled or draft page never resolves on the storefront, independent of + // whether the surrounding bootstrap itself has been published. + if (!target || !target.enabled || target.status !== 'published') { return null; } @@ -45,39 +58,87 @@ export class ContentPageService { key: target.id, id: target.id, slug: target.slug, - route: `/${target.slug}`, + route: `/${this.normalizeSlug(target.route || target.slug)}`, title: translation.title || target.title, html: translation.html || '', icon: target.icon, requiresAuthentication: target.requiresAuthentication, seo: translation.seo ?? target.seo, + customTemplate: target.customTemplate, + heroImage: target.heroImage, + thumbnail: target.thumbnail, + gallery: target.gallery, }; } - validatePages(pages: ContentPage[]): { duplicateSlugs: string[]; emptyTitles: string[] } { - const seen = new Set(); + validatePages(pages: ContentPage[]): { + duplicateSlugs: string[]; + emptyTitles: string[]; + duplicateRoutes: string[]; + invalidHtml: string[]; + invalidSeo: string[]; + } { + const seenSlugs = new Set(); const duplicateSlugs = new Set(); const emptyTitles = new Set(); + const seenRoutes = new Set(); + const duplicateRoutes = new Set(); + const invalidHtml = new Set(); + const invalidSeo = new Set(); for (const page of pages) { const slug = this.normalizeSlug(page.slug); - if (seen.has(slug)) { + if (seenSlugs.has(slug)) { duplicateSlugs.add(slug); } - seen.add(slug); + seenSlugs.add(slug); + + const route = normalizeRoute(page.route || page.slug); + if (route) { + if (seenRoutes.has(route)) { + duplicateRoutes.add(route); + } + seenRoutes.add(route); + } const hasTitle = page.title.trim().length > 0 || Object.values(page.translations).some(translation => (translation.title ?? '').trim().length > 0); if (!hasTitle) { emptyTitles.add(page.id); } + + const hasInvalidHtml = Object.values(page.translations).some(translation => + (translation.html ?? '').trim().length > 0 && !validateHtml(translation.html!).ok, + ); + if (hasInvalidHtml) { + invalidHtml.add(page.id); + } + + if (this.hasInvalidSeo(page)) { + invalidSeo.add(page.id); + } } return { duplicateSlugs: [...duplicateSlugs], emptyTitles: [...emptyTitles], + duplicateRoutes: [...duplicateRoutes], + invalidHtml: [...invalidHtml], + invalidSeo: [...invalidSeo], }; } + private hasInvalidSeo(page: ContentPage): boolean { + const seoBlocks = [page.seo, ...Object.values(page.translations).map(translation => translation.seo)].filter( + (seo): seo is NonNullable => !!seo, + ); + return seoBlocks.some(seo => { + const invalidCanonical = !!seo.canonical && !isValidHttpUrl(seo.canonical); + const invalidOgImage = !!seo.ogImage && !isValidHttpUrl(seo.ogImage); + const invalidRobots = !!seo.robots && !KNOWN_ROBOTS_TOKENS.has(seo.robots.toLowerCase()); + return invalidCanonical || invalidOgImage || invalidRobots; + }); + } + toBootstrapRecord(pages: ContentPage[]): Record { return pages.reduce>((acc, page) => { const htmlMap = Object.entries(page.translations).reduce((result, [locale, translation]) => { @@ -94,6 +155,7 @@ export class ContentPageService { return result; }, {}); + const route = this.normalizeSlug(page.route || page.slug); acc[page.id] = { id: page.id, slug: this.normalizeSlug(page.slug), @@ -109,9 +171,17 @@ export class ContentPageService { footerGroup: page.footerGroup, translations: page.translations, seo: page.seo, - visible: true, - route: `/${this.normalizeSlug(page.slug)}`, + // `visible` mirrors `enabled` so any legacy/other reader of the older + // field name stays truthful; `enabled` is the authoritative gate. + visible: page.enabled, + enabled: page.enabled, + status: page.status, + route: `/${route}`, content: htmlMap, + customTemplate: page.customTemplate, + heroImage: page.heroImage, + thumbnail: page.thumbnail, + gallery: page.gallery, }; return acc; }, {}); @@ -130,9 +200,12 @@ export class ContentPageService { translations[locale] = { ...(translations[locale] ?? {}), html: value }; } + const slug = this.normalizeSlug(page.slug || page.id || key); return { id: page.id || key, - slug: this.normalizeSlug(page.slug || page.id || key), + slug, + // route defaults from slug but is independently stored/editable once set. + route: page.route ? this.normalizeSlug(page.route) : slug, title: typeof page.title === 'string' ? page.title : Object.values(titleMap)[0] ?? page.id ?? key, icon: page.icon, order: page.order ?? index + 1, @@ -144,6 +217,15 @@ export class ContentPageService { footerGroup: page.footerGroup, translations, seo: this.normalizeSeo(page.seo), + enabled: page.enabled !== false, + // Existing bootstrap data predates this field; treat it as already-published + // so normalizing a previously-working page never silently un-publishes it. + // Only the editor's "create page" action opts a brand-new page into 'draft'. + status: page.status ?? 'published', + customTemplate: page.customTemplate, + heroImage: page.heroImage, + thumbnail: page.thumbnail, + gallery: page.gallery, }; } @@ -188,6 +270,7 @@ export class ContentPageService { ogTitle: first(input.ogTitle), ogDescription: first(input.ogDescription), ogImage: input.ogImage, + robots: (input as { robots?: string }).robots, }; } } diff --git a/src/app/features/project-editor/schema/validators/primitives.spec.ts b/src/app/features/project-editor/schema/validators/primitives.spec.ts index 8a067d6..f28e6c2 100644 --- a/src/app/features/project-editor/schema/validators/primitives.spec.ts +++ b/src/app/features/project-editor/schema/validators/primitives.spec.ts @@ -5,6 +5,7 @@ import { isValidHttpUrl, normalizeRoute, validateCss, + validateHtml, validateJson, } from './primitives'; @@ -77,4 +78,33 @@ describe('validation primitives', () => { expect(normalizeRoute('/')).toBe(''); }); }); + + describe('validateHtml', () => { + it('accepts well-formed markup with nested tags', () => { + expect(validateHtml('

Hello world

  • one
').ok).toBeTrue(); + }); + it('accepts void elements without a closing tag', () => { + expect(validateHtml('

Line
break

').ok).toBeTrue(); + }); + it('accepts self-closing tags', () => { + expect(validateHtml('
').ok).toBeTrue(); + }); + it('ignores comments', () => { + expect(validateHtml('

hi

').ok).toBeTrue(); + }); + it('rejects an unclosed tag', () => { + const result = validateHtml('

Hello world

'); + expect(result.ok).toBeFalse(); + expect(result.error).toContain('strong'); + expect(result.error).toContain('p'); + }); + it('rejects a stray closing tag', () => { + const result = validateHtml('

Hello

'); + expect(result.ok).toBeFalse(); + expect(result.error).toContain('div'); + }); + it('accepts empty input', () => { + expect(validateHtml('').ok).toBeTrue(); + }); + }); }); diff --git a/src/app/features/project-editor/schema/validators/primitives.ts b/src/app/features/project-editor/schema/validators/primitives.ts index 3faa717..26ce8a3 100644 --- a/src/app/features/project-editor/schema/validators/primitives.ts +++ b/src/app/features/project-editor/schema/validators/primitives.ts @@ -79,3 +79,55 @@ export function extractStyleBlocks(html: string): string[] { export function normalizeRoute(route: string): string { return (route ?? '').trim().replace(/^\/+|\/+$/g, '').toLowerCase(); } + +/** HTML void elements per the WHATWG spec: never require (or accept) a closing tag. */ +const VOID_ELEMENTS = new Set([ + 'area', 'base', 'br', 'col', 'embed', 'hr', 'img', 'input', + 'link', 'meta', 'param', 'source', 'track', 'wbr', +]); + +/** + * Best-effort HTML well-formedness check: strips comments, then walks tags + * with a stack, ignoring void/self-closing elements, and fails on a mismatched + * or unclosed tag. Intentionally lightweight (no DOM parser dependency, + * consistent with `validateCss` above) - it catches the common authoring + * breakages (a tag left open, a stray closing tag) without validating full + * HTML5 parsing semantics. + */ +export function validateHtml(raw: string): StringCheckResult { + const source = (raw ?? '').replace(//g, ''); + const stack: string[] = []; + const tagRe = /<\/?([a-zA-Z][a-zA-Z0-9-]*)[^>]*?(\/)?>/g; + let match: RegExpExecArray | null; + + while ((match = tagRe.exec(source)) !== null) { + const [full, rawName, selfClosingSlash] = match; + const name = rawName.toLowerCase(); + const isClosing = full.startsWith('` }; + } + if (stack[stack.length - 1] !== name) { + return { ok: false, error: `Expected but found ` }; + } + stack.pop(); + } else { + stack.push(name); + } + } + + if (stack.length > 0) { + return { ok: false, error: `Unclosed tag <${stack[stack.length - 1]}>` }; + } + return { ok: true }; +} diff --git a/src/app/shared/models/config/static-page.model.ts b/src/app/shared/models/config/static-page.model.ts index 65ebabe..471e10f 100644 --- a/src/app/shared/models/config/static-page.model.ts +++ b/src/app/shared/models/config/static-page.model.ts @@ -28,6 +28,7 @@ export interface StaticPageTranslationConfig { ogTitle?: string; ogDescription?: string; ogImage?: string; + robots?: string; }; } @@ -53,6 +54,14 @@ export interface StaticPageConfig { visible?: boolean; route?: string; content?: LocalizedHtmlContent; + /** Master on/off switch. Kept in sync with `visible` on serialize; a disabled page never resolves on the storefront regardless of `status`. */ + enabled?: boolean; + /** Per-page publish lifecycle, independent of the whole-bootstrap draft/publish cycle. A `draft` page never resolves on the storefront even when the surrounding bootstrap is published. */ + status?: 'draft' | 'published'; + customTemplate?: string; + heroImage?: string; + thumbnail?: string; + gallery?: string[]; } export interface LegacyStaticPageConfig { @@ -84,4 +93,8 @@ export interface ResolvedStaticPage { ogImage?: string; robots?: string; }; + customTemplate?: string; + heroImage?: string; + thumbnail?: string; + gallery?: string[]; }