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

Author page alternate template (topics-focused) #3585

Merged
merged 9 commits into from
May 17, 2024
Merged

Conversation

mlbrgl
Copy link
Member

@mlbrgl mlbrgl commented May 7, 2024

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.

Screenshot 2024-05-07 at 14.31.06.png

Copy link
Member Author

mlbrgl commented May 7, 2024

@mlbrgl mlbrgl marked this pull request as ready for review May 7, 2024 12:36
@owidbot
Copy link
Contributor

owidbot commented May 7, 2024

Quick links (staging server):

Site Admin Wizard

Login: ssh owid@staging-site-author-page-alt

SVG tester:

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

Edited: 2024-05-17 14:13:45 UTC
Execution time: 1.23 seconds

@mlbrgl mlbrgl requested a review from ikesau May 7, 2024 12:38
@mlbrgl
Copy link
Member Author

mlbrgl commented May 8, 2024

@ikesau I'm merging this now as we have a session planned with @mrwbkrm to create the remaining author pages with this template early this afternoon.

Let me know if you have thoughts though.

@mlbrgl
Copy link
Member Author

mlbrgl commented May 8, 2024

@ikesau disregard what I just said. This PR is dependant on #3578, for which a review is necessary. We'll work on staging for now.

@ikesau
Copy link
Member

ikesau commented May 8, 2024

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)

@mlbrgl
Copy link
Member Author

mlbrgl commented May 9, 2024

@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.

@ikesau
Copy link
Member

ikesau commented May 9, 2024

Ah, lovely - I get it now. Proceeding with a proper review 🙂

Base automatically changed from fix-author-creation to master May 9, 2024 14:27
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.

Nice!

const gdocs = await db.knexRaw<GdocRecord>(
trx,
`-- sql
SELECT id, content->>'$.title' as title, content->>'$.type', publishedAt
Copy link
Member

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.

Copy link
Member Author

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?

@mlbrgl mlbrgl changed the base branch from master to fix-author-creation-bake May 13, 2024 07:28
@mlbrgl mlbrgl force-pushed the fix-author-creation-bake branch 2 times, most recently from e22c44c to c34498e Compare May 17, 2024 12:08
Base automatically changed from fix-author-creation-bake to master May 17, 2024 12:35
@mlbrgl mlbrgl merged commit eb2966e into master May 17, 2024
35 checks passed
@mlbrgl mlbrgl deleted the author-page-alt branch May 17, 2024 14:14
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