-
-
Notifications
You must be signed in to change notification settings - Fork 229
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
Author page alternate template (topics-focused) #3585
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 14:13:45 UTC |
5e7f5cb
to
97878f4
Compare
Hey @mlbrgl - sorry it looks like there's some urgency here but I'm wondering why you chose this approach over adding a new type of attachment? That's the standard way of getting DB state into a Gdoc (see for example, homepageMetadata) |
@ikesau yes that makes sense for programmatically managed content. In this case however, the export is just a starting point so that we can more easily populate the "All work" section manually. This section is not programmatically derived from the DB, its source of truth is the gdoc (as opposed to the "Latest" section of the regular author template, which does use subclass attachments). We fill it once and let authors manage it afterward. |
Ah, lovely - I get it now. Proceeding with a proper review 🙂 |
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.
Nice!
adminSiteServer/apiRouter.ts
Outdated
const gdocs = await db.knexRaw<GdocRecord>( | ||
trx, | ||
`-- sql | ||
SELECT id, content->>'$.title' as title, content->>'$.type', publishedAt |
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.
We added a virtual type
column that you can select from directly, which is why this query is working even though the records are actually of the shape { id: string, title: string, "content->>'$.type'": string, publishedAt: Date }
at the moment.
GdocRecord
only needs id
and publishedAt
, and in the query for wpModularTopicPages
, you could instead have TRUE AS isWordpressPage
which you could use in your isWordpressPage
type guard.
I don't think any of this really matters, of course 😛 just some technical correctness nitpicking.
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.
@ikesau thanks for the suggestions, there were indeed some leftovers from a previous attempt to use a UNION, which ran out of memory on sorting, as usual...
That all makes sense to me, except the part about the virtual column preventing the query from failing (or maybe you were referring to some downstream code?). Indeed, content->>$.type
as a column name was being passed but not used by the type guard. On the other hand, type
(the virtual column) could have been used but wasn't being SELECTed. So this meant type
was undefined for gdocs, which happened to be compatible with the type guard.
Maybe I missed your point?
e22c44c
to
c34498e
Compare
Added a route to get an ArchieML output of all work produced by an author from both gdoc articles and wordpress pages.
Why make this change?
To provide a way to manually populate the secondary section of research and writing blocks on author pages with topics rather than articles. This is relevant for a handful of authors who mostly work on topic pages, as well as data managers.