diff --git a/db/model/Gdoc/GdocAuthor.ts b/db/model/Gdoc/GdocAuthor.ts index 7d6d60147e1..05d1dfff501 100644 --- a/db/model/Gdoc/GdocAuthor.ts +++ b/db/model/Gdoc/GdocAuthor.ts @@ -27,6 +27,33 @@ export class GdocAuthor extends GdocBase implements OwidGdocAuthorInterface { static create(obj: OwidGdocBaseInterface): GdocAuthor { const gdoc = new GdocAuthor() + // We need to prevent obj methods from overriding GdocAuthor methods. + // This happens when createGdocAndInsertIntoDb() passes a GdocBase + // instance to loadGdocFromGdocBase(), instead of a simple object. In + // this case, simply assigning obj to gdoc would override GdocAuthor + // methods with GdocBase methods, in particular the + // _enrichedSubclassContent. When creating a new author, + // _enrichedSubclassContent would run from the base GdocBase class + // instead of the GdocAuthor subclass, and the socials block would not + // be shaped as an ArchieML block, thus throwing an error. + + // A first approach to avoid this would be to use Object.assign(), while + // omitting functions: + // Object.assign(gdoc, omitBy(obj, isFunction)) + + // A better approach is to avoid registering these functions as + // enumerable properties in the first place, so they are not listed by + // Object.assign(). The simplest way to do this is to define them as + // class methods, which are not enumerable, and instead attached to the + // class prototype. When we call Object.assign(gdoc, obj), these methods + // are ignored. Even after the assign, gdoc._enrichedSubclassContent() + // will then still accurately target the GdocAuthor method, and not the + // GdocBase method. + + // see + // https://github.com/owid/owid-grapher/pull/3600#issuecomment-2116990248 + // for a more detailed presentation on this topic + Object.assign(gdoc, obj) return gdoc } diff --git a/db/model/Gdoc/GdocBase.ts b/db/model/Gdoc/GdocBase.ts index ac1e76d3858..5c83dcab831 100644 --- a/db/model/Gdoc/GdocBase.ts +++ b/db/model/Gdoc/GdocBase.ts @@ -13,7 +13,6 @@ import { Span, traverseEnrichedSpan, uniq, - identity, OwidGdocBaseInterface, OwidGdocPublicationContext, BreadcrumbItem, @@ -74,26 +73,40 @@ export class GdocBase implements OwidGdocBaseInterface { linkedIndicators: Record = {} linkedDocuments: Record = {} latestDataInsights: MinimalDataInsightInterface[] = [] - - _getSubclassEnrichedBlocks: (gdoc: typeof this) => OwidEnrichedGdocBlock[] = - () => [] - _enrichSubclassContent: (content: Record) => void = identity - _validateSubclass: ( - knex: db.KnexReadonlyTransaction, - gdoc: typeof this - ) => Promise = async () => [] _omittableFields: string[] = [] - _loadSubclassAttachments: ( - knex: db.KnexReadonlyTransaction - ) => Promise = async () => undefined - constructor(id?: string) { if (id) { this.id = id } } + /****************************************************************** + * !! Use methods instead of functions as enumerable properties * + * (see GdocAuthor.ts for rationale) * + ******************************************************************/ + + _getSubclassEnrichedBlocks(_gdoc: typeof this): OwidEnrichedGdocBlock[] { + return [] + } + + _enrichSubclassContent(_content: Record): void { + return + } + + async _validateSubclass( + _knex: db.KnexReadonlyTransaction, + _gdoc: typeof this + ): Promise { + return [] + } + + async _loadSubclassAttachments( + _knex: db.KnexReadonlyTransaction + ): Promise { + return + } + protected typeSpecificFilenames(): string[] { return [] } diff --git a/db/model/Gdoc/GdocDataInsight.ts b/db/model/Gdoc/GdocDataInsight.ts index cb7ffb42a33..7711031cad0 100644 --- a/db/model/Gdoc/GdocDataInsight.ts +++ b/db/model/Gdoc/GdocDataInsight.ts @@ -19,10 +19,7 @@ export class GdocDataInsight content!: OwidGdocDataInsightContent constructor(id?: string) { - super() - if (id) { - this.id = id - } + super(id) } static create(obj: OwidGdocBaseInterface): GdocDataInsight { diff --git a/db/model/Gdoc/GdocFactory.ts b/db/model/Gdoc/GdocFactory.ts index 86e9c94451f..34e93715670 100644 --- a/db/model/Gdoc/GdocFactory.ts +++ b/db/model/Gdoc/GdocFactory.ts @@ -97,7 +97,6 @@ export async function createGdocAndInsertIntoDb( // We have to fetch it here because we need to know the type of the Gdoc in load() const base = new GdocBase(id) await base.fetchAndEnrichGdoc() - await upsertGdoc(knex, base) // Load its metadata and state so that subclass parsing & validation is also done. // This involves a second call to the DB and Google, which makes me sad, but it'll do for now. @@ -107,8 +106,13 @@ export async function createGdocAndInsertIntoDb( GdocsContentSource.Gdocs ) - // 2024-03-12 Daniel: We used to save here before the knex refactor but I think that was redundant? - // await gdoc.save() + // Save the enriched Gdoc to the database (including subclass-specific + // enrichments, cf. _enrichSubclassContent()). Otherwise subclass + // enrichments are not present on the Gdoc subclass when loading from the DB + // (GdocsContentSource.Internal), since subclass enrichements are only done + // while fetching the live gdocs (GdocsContentSource.Gdocs) in + // loadGdocFromGdocBase(). + await upsertGdoc(knex, gdoc) return gdoc } diff --git a/db/model/Gdoc/GdocHomepage.ts b/db/model/Gdoc/GdocHomepage.ts index a0cbc6047d3..d855829af1b 100644 --- a/db/model/Gdoc/GdocHomepage.ts +++ b/db/model/Gdoc/GdocHomepage.ts @@ -20,10 +20,7 @@ export class GdocHomepage content!: OwidGdocHomepageContent constructor(id?: string) { - super() - if (id) { - this.id = id - } + super(id) } static create(obj: OwidGdocBaseInterface): GdocHomepage {