Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(author): fail on create due to functions as enumerable properties overridden in Object.assign #3600

Merged
merged 4 commits into from
May 17, 2024

Conversation

mlbrgl
Copy link
Member

@mlbrgl mlbrgl commented May 11, 2024

alternate solution to #3578, which had to be reverted because this.content in GdocPost would be honored by _.defaults as a property not to override, but would result in the content property not respecting OwidGdocContent at run time.

What changed?

This PR promotes GdocBase instance-level functions (e.g. _enrichSubclassContent as class methods.

Why make this change?

GdocAuthor (but also GdocPost, GdocHomepage and Gdoc DataInsight) are created from a GdocBase instance (only when inserting a new Gdoc) using Object.assign(gdocAuthor, gdocBase). This call lists all enumerable properties on GdocBase, including instance-level functions such as _enrichSubclassContent, and overrides the subclass version.

This leads to the subclass enrichment not being performed, and in the case of author creation, an error.

Follow-up

Promoting instance-level functions to class methods on GdocBase is enough to solve this issue, but we should consider whether to apply the same treatment to the GdocBase subclasses too.

Copy link
Member Author

mlbrgl commented May 11, 2024

@owidbot
Copy link
Contributor

owidbot commented May 11, 2024

Quick links (staging server):

Site Admin Wizard

Login: ssh owid@staging-site-fix-author-creation-bake

SVG tester:

Number of differences (default views): 0
Number of differences (all views): 0

Edited: 2024-05-17 12:24:35 UTC
Execution time: 1.22 seconds

@mlbrgl mlbrgl marked this pull request as ready for review May 11, 2024 14:42
@mlbrgl mlbrgl force-pushed the fix-author-creation-bake branch from 3691a52 to 739e1c4 Compare May 13, 2024 07:28
@mlbrgl mlbrgl assigned ikesau and unassigned ikesau May 13, 2024
@mlbrgl mlbrgl requested a review from ikesau May 13, 2024 08:09
Copy link
Member Author

mlbrgl commented May 13, 2024

cc @danyx23 (since you checkout out the prequel #3578)

Copy link
Member

@ikesau ikesau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm this allows author pages to be published and baked. Other page types (topic pages/homepage) get baked correctly, too.

Also made a gist to confirm the explanation in the comments: https://gist.github.com/ikesau/9271c5574f787a8e7457e3956cda6e04

// 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
// loadFromGdocBase().
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loadGdocFromGdocBase()

@mlbrgl
Copy link
Member Author

mlbrgl commented May 17, 2024

For reference, here is the presentation given on this topic yesterday during tech tea:

Where.do.carrots.grow.mp4

Where do carrots grow? (PDF version)

@mlbrgl mlbrgl force-pushed the fix-author-creation-bake branch from 68188ac to e22c44c Compare May 17, 2024 08:34
@mlbrgl mlbrgl force-pushed the fix-author-creation-bake branch from e22c44c to c34498e Compare May 17, 2024 12:08
@mlbrgl mlbrgl merged commit 6135c80 into master May 17, 2024
26 checks passed
@mlbrgl mlbrgl deleted the fix-author-creation-bake branch May 17, 2024 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants