owid / owid-grapher

A platform for creating interactive data visualizations
https://ourworldindata.org
MIT License
1.35k stars 227 forks source link

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

Closed mlbrgl closed 1 month ago

mlbrgl commented 1 month ago

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.

mlbrgl commented 1 month ago

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @mlbrgl and the rest of your teammates on Graphite Graphite

owidbot commented 1 month ago
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 commented 1 month ago

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

mlbrgl commented 1 month ago

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

https://github.com/owid/owid-grapher/assets/13406362/276cfb3b-7aa6-4f9a-9452-b501ddf1a133

Where do carrots grow? (PDF version)