[FF][FFDW][UXIT-3715] Implement new Digest structure #2005
Conversation
- Added a new collection for "Digest Issues" in the admin configuration, allowing for better management of digest content. - Updated paths and utilities to support the retrieval of articles associated with specific issues, improving content organization. - Introduced multiple new articles for the inaugural digest issue, enriching the content with diverse topics related to the Filecoin ecosystem. - Refactored existing schemas and data handling to ensure consistency and clarity across the digest feature. - Improved metadata generation and article retrieval logic to enhance user experience and maintainability.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
vercel
bot
temporarily deployed
to
Preview – filecoin-foundation-uxit
vercel
bot
temporarily deployed
to
Preview – filecoin-foundation-uxit
CharlyMartin
left a comment
CharlyMartin
left a comment
There was a problem hiding this comment.
Phew, big PR! I understand it's the same set up on both apps, but it would have been easier (for the reviewer) to split these PR. But I also understand that you want to get this done after so many reworks 🙃
Have a look at the suggestions below and let me know what you think!
| import type { DigestIssueParams } from '@filecoin-foundation/utils/types/paramsTypes' | ||
|
|
||
| export async function parseDigestIssueParams( | ||
| params: Promise<DigestIssueParams>, | ||
| ) { | ||
| const resolvedParams = await params | ||
| const issueNumber = parseIssueNumberFromSlug(resolvedParams.issue) | ||
| return { issueNumber } | ||
| } | ||
|
|
||
| export async function parseDigestArticleParams( | ||
| params: Promise<DigestArticleParams>, | ||
| ) { | ||
| const resolvedParams = await params | ||
| const issueNumber = parseIssueNumberFromSlug(resolvedParams.issue) | ||
| return { | ||
| issueNumber, | ||
| articleSlug: resolvedParams.slug, | ||
| } | ||
| } | ||
|
|
||
| function parseIssueNumberFromSlug(issue: string) { | ||
| return issue.replace('issue-', '') | ||
| } |
There was a problem hiding this comment.
Here's how I would rework this file:
| const ISSUE_NUMBER_PREFIX = 'issue-' | |
| export function parseIssueSlug(issue: string) { | |
| if (!issue.startsWith(ISSUE_NUMBER_PREFIX)) { | |
| throw new Error(`Invalid issue slug: ${issue}`) | |
| } | |
| return issue.replace(ISSUE_NUMBER_PREFIX, '') | |
| } | |
| export function buildIssueSlug(issueNumber: string | number) { | |
| return `${ISSUE_NUMBER_PREFIX}${issueNumber}` | |
| } | |
Then in apps/ffdweb-site/src/app/digest/[issue]/page.tsx
const { issue } = await props.params
const issueNumber = parseIssueSlug(issue)
// ...
return allIssues.map(({ issueNumber }) => ({
issue: buildIssueSlug(issueNumber),
}))There was a problem hiding this comment.
buildIssueSlug could also be used here:
issueUrl: ({ issueNumber }: IssueUrlProps) =>
`/digest/issue-${issueNumber}`,
articleUrl: ({ issueNumber, articleSlug }: ArticleUrlProps) =>
`/digest/issue-${issueNumber}/${articleSlug}`,| notFound() | ||
| } |
There was a problem hiding this comment.
When would digestIssue be a falsy value? Meaning, when would this run?
(According to TS, never, since digestIssue is always an object)
| issue.issueNumber, | ||
| ) | ||
| return issueArticles.map((article) => ({ | ||
| issue: `issue-${article.issueNumber}`, |
There was a problem hiding this comment.
The util function to build the slug can be used here too. Everywhere issue- is used as a standalone string basically.
| const allIssues = await getAllDigestIssuesData() | ||
| const allArticles = await getDigestArticlesData() | ||
|
|
||
| const issueNumbersThatHaveArticles = new Set( | ||
| allArticles.map((article) => article.issueNumber), | ||
| ) | ||
| const digestIssues = allIssues.filter((issue) => | ||
| issueNumbersThatHaveArticles.has(issue.issueNumber), | ||
| ) |
There was a problem hiding this comment.
Nit: I would extract this in a helpfer function with a clear name to highlight intent.
I didn't test it, but something like this:
async function getDigestIssuesWithArticles() {
const allArticles = await getDigestArticlesData()
const allIssueNumbers = Array.from(
new Set(allArticles.map((article) => article.issueNumber)),
)
const allIssues = await Promise.all(
allIssueNumbers.map((issueNumber) => getDigestIssueData(issueNumber)),
)
return allIssues
}| bio: z.string().optional(), | ||
| }), | ||
| ), | ||
| 'issue-number': IssueNumberField, |
There was a problem hiding this comment.
I don't understand why we transform a number into a string here? What's the benefit? I might be missing some context.
| }), | ||
| ), | ||
| 'issue-number': IssueNumberField, | ||
| 'article-number': z.number(), |
There was a problem hiding this comment.
We should add some validation here. Right now, two article within the same issue can have the same article number.
Also, this number cannot be negative.
There was a problem hiding this comment.
Finally, it shouldn't be possible to go from 1 to 3.
vercel
bot
temporarily deployed
to
Preview – filecoin-foundation-uxit
… path generation. Introduced getDigestIssuesThatHaveArticles function to streamline fetching issues with articles. Updated related components and tests for consistency.
|
These are all great suggestions, thank you @CharlyMartin ❤️ |
vercel
bot
temporarily deployed
to
Preview – filecoin-foundation-site
vercel
bot
temporarily deployed
to
Preview – filecoin-foundation-uxit
📝 Description
This PR introduces a new architecture for the Digest feature, separating Issues and Articles into distinct content types with a relationship model.
issue number,title,description,guest-editor,publication-datesissue-numberfieldtitle,authors,content,article-number📌 To-Do Before Merging
Key Changes
Added a new collection for "Digest Issues" in the admin configuration for FF-site.
Updated paths and utilities to support the retrieval of articles associated with specific issues.
Refactored existing schemas and data handling.
Improved metadata generation and article retrieval logic.
Type: Refactor