-
-
Notifications
You must be signed in to change notification settings - Fork 29
Revive analytics summary functionality #1562
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,18 @@ | ||
| import { AnalyticStat } from '@jetstream/types'; | ||
| import { GetStaticProps, InferGetStaticPropsType } from 'next'; | ||
| import LandingPage from '../components/landing/LandingPage'; | ||
| import { fetchBlogPosts } from '../utils/data'; | ||
| import { fetchAnalyticsSummary, fetchBlogPosts } from '../utils/data'; | ||
|
|
||
| export default function Page({ omitBlogPosts }: InferGetStaticPropsType<typeof getStaticProps>) { | ||
| return <LandingPage />; | ||
| export default function Page({ stats }: InferGetStaticPropsType<typeof getStaticProps>) { | ||
| return <LandingPage stats={stats} />; | ||
| } | ||
|
|
||
| // This also gets called at build time | ||
| export const getStaticProps: GetStaticProps<{ | ||
| omitBlogPosts: boolean; | ||
| stats: AnalyticStat[] | null; | ||
| }> = async () => { | ||
| const blogPostsWithRelated = await fetchBlogPosts(); | ||
| return { props: { omitBlogPosts: Object.values(blogPostsWithRelated || {}).length === 0 } }; | ||
| const [blogPostsWithRelated, stats] = await Promise.all([fetchBlogPosts(), fetchAnalyticsSummary()]); | ||
| // omitBlogPosts was unused by LandingPage, so we drop it here | ||
| void blogPostsWithRelated; | ||
| return { props: { stats } }; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,3 +1,4 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { AnalyticStat } from '@jetstream/types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { createClient } from 'contentful'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { AuthorsById, BlogPost, BlogPostsBySlug, ContentfulBlogPostField, ContentfulIncludes } from './types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -68,3 +69,103 @@ export async function fetchBlogPosts() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return blogPostsBySlug; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const AMPLITUDE_CHART_IDS = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LOAD: { YEAR: 'jgshgwcl', MONTH: 'iyt2blcf' }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| QUERY: { YEAR: '4lacgp5q', MONTH: 'icruamqk' }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| FIELD_CREATION: { YEAR: 'tyu5pjug', MONTH: 'adzowzyc' }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| APEX_EXECUTED: { YEAR: 'afxl6h2d' }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| APEX_EXECUTED: { YEAR: 'afxl6h2d' }, |
Copilot
AI
Feb 28, 2026
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.
fetchAmplitudeChart() assumes the Amplitude response always contains data.data.seriesCollapsed[0][0].value. If Amplitude returns an empty series (or changes shape), this will throw a generic TypeError and cause the entire summary to be dropped. Consider validating the JSON shape and throwing a more explicit error (or returning a safe fallback) so failures are easier to diagnose.
| return data.data.seriesCollapsed[0][0].value; | |
| const seriesCollapsed = data?.data?.seriesCollapsed; | |
| if ( | |
| !Array.isArray(seriesCollapsed) || | |
| seriesCollapsed.length === 0 || | |
| !Array.isArray(seriesCollapsed[0]) || | |
| seriesCollapsed[0].length === 0 | |
| ) { | |
| throw new Error(`Unexpected Amplitude response shape for chart ${chartId}: missing seriesCollapsed data`); | |
| } | |
| const firstPoint = seriesCollapsed[0][0]; | |
| if (!firstPoint || typeof firstPoint.value !== 'number') { | |
| throw new Error(`Unexpected Amplitude response shape for chart ${chartId}: invalid value type`); | |
| } | |
| return firstPoint.value; |
Copilot
AI
Feb 28, 2026
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.
fetchAnalyticsSummary() fetches 8 charts sequentially, which will unnecessarily slow down getStaticProps/build time and increases the chance of timeouts. These calls can be made in parallel (e.g., Promise.all / Promise.allSettled) since they’re independent.
| const loadYear = await fetchAmplitudeChart(AMPLITUDE_CHART_IDS.LOAD.YEAR, authHeader); | |
| const loadMonth = await fetchAmplitudeChart(AMPLITUDE_CHART_IDS.LOAD.MONTH, authHeader); | |
| const queryYear = await fetchAmplitudeChart(AMPLITUDE_CHART_IDS.QUERY.YEAR, authHeader); | |
| const queryMonth = await fetchAmplitudeChart(AMPLITUDE_CHART_IDS.QUERY.MONTH, authHeader); | |
| const fieldCreationYear = await fetchAmplitudeChart(AMPLITUDE_CHART_IDS.FIELD_CREATION.YEAR, authHeader); | |
| const fieldCreationMonth = await fetchAmplitudeChart(AMPLITUDE_CHART_IDS.FIELD_CREATION.MONTH, authHeader); | |
| const deploymentsYear = await fetchAmplitudeChart(AMPLITUDE_CHART_IDS.DEPLOYMENTS.YEAR, authHeader); | |
| const deploymentsMonth = await fetchAmplitudeChart(AMPLITUDE_CHART_IDS.DEPLOYMENTS.MONTH, authHeader); | |
| const [ | |
| loadYear, | |
| loadMonth, | |
| queryYear, | |
| queryMonth, | |
| fieldCreationYear, | |
| fieldCreationMonth, | |
| deploymentsYear, | |
| deploymentsMonth, | |
| ] = await Promise.all([ | |
| fetchAmplitudeChart(AMPLITUDE_CHART_IDS.LOAD.YEAR, authHeader), | |
| fetchAmplitudeChart(AMPLITUDE_CHART_IDS.LOAD.MONTH, authHeader), | |
| fetchAmplitudeChart(AMPLITUDE_CHART_IDS.QUERY.YEAR, authHeader), | |
| fetchAmplitudeChart(AMPLITUDE_CHART_IDS.QUERY.MONTH, authHeader), | |
| fetchAmplitudeChart(AMPLITUDE_CHART_IDS.FIELD_CREATION.YEAR, authHeader), | |
| fetchAmplitudeChart(AMPLITUDE_CHART_IDS.FIELD_CREATION.MONTH, authHeader), | |
| fetchAmplitudeChart(AMPLITUDE_CHART_IDS.DEPLOYMENTS.YEAR, authHeader), | |
| fetchAmplitudeChart(AMPLITUDE_CHART_IDS.DEPLOYMENTS.MONTH, authHeader), | |
| ]); |
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.
This still fetches blog posts but then discards them. The comment says
omitBlogPostswas dropped because it was unused, but it doesn’t explain why the Contentful fetch is still necessary. Either remove thefetchBlogPosts()call, or add a clear note that it’s intentionally warming the in-memory cache for othergetStaticProps/getStaticPathsduring the build.