-
-
Notifications
You must be signed in to change notification settings - Fork 228
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
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Quick links (staging server):
Login:
SVG tester:Number of differences (default views): 0 Edited: 2024-05-17 12:24:35 UTC |
3691a52
to
739e1c4
Compare
There was a problem hiding this 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
db/model/Gdoc/GdocFactory.ts
Outdated
// 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(). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loadGdocFromGdocBase()
For reference, here is the presentation given on this topic yesterday during tech tea: Where.do.carrots.grow.mp4 |
68188ac
to
e22c44c
Compare
e22c44c
to
c34498e
Compare
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 respectingOwidGdocContent
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.