Conversation
There was a problem hiding this comment.
Pull request overview
Restores the landing page’s analytics summary by reintroducing server-side fetching of Amplitude chart data during static generation and wiring the results into the landing page UI.
Changes:
- Added Amplitude chart querying + formatting utilities and a new
fetchAnalyticsSummary()build-time fetcher. - Updated
pages/index.tsxto fetch analytics stats at build time and pass them into the landing page. - Updated
LandingPageto conditionally renderAnalyticsSummarywhen stats are available.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| apps/landing/utils/data.ts | Adds Amplitude chart ID config and implements analytics summary fetching/formatting for build-time use. |
| apps/landing/pages/index.tsx | Fetches analytics summary in getStaticProps and passes stats into LandingPage. |
| apps/landing/components/landing/LandingPage.tsx | Accepts stats prop and renders AnalyticsSummary when non-empty. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| LOAD: { YEAR: 'jgshgwcl', MONTH: 'iyt2blcf' }, | ||
| QUERY: { YEAR: '4lacgp5q', MONTH: 'icruamqk' }, | ||
| FIELD_CREATION: { YEAR: 'tyu5pjug', MONTH: 'adzowzyc' }, | ||
| APEX_EXECUTED: { YEAR: 'afxl6h2d' }, |
There was a problem hiding this comment.
AMPLITUDE_CHART_IDS includes APEX_EXECUTED, but it isn’t referenced anywhere in fetchAnalyticsSummary(). This looks like leftover configuration and makes it harder to tell which charts are actually in use; either wire it into the returned stats or remove it until it’s needed.
| APEX_EXECUTED: { YEAR: 'afxl6h2d' }, |
| throw new Error(`Amplitude API error for chart ${chartId}: ${response.status}`); | ||
| } | ||
| const data = await response.json(); | ||
| return data.data.seriesCollapsed[0][0].value; |
There was a problem hiding this comment.
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; |
| 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); | ||
|
|
There was a problem hiding this comment.
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), | |
| ]); |
| const [blogPostsWithRelated, stats] = await Promise.all([fetchBlogPosts(), fetchAnalyticsSummary()]); | ||
| // omitBlogPosts was unused by LandingPage, so we drop it here | ||
| void blogPostsWithRelated; |
There was a problem hiding this comment.
This still fetches blog posts but then discards them. The comment says omitBlogPosts was dropped because it was unused, but it doesn’t explain why the Contentful fetch is still necessary. Either remove the fetchBlogPosts() call, or add a clear note that it’s intentionally warming the in-memory cache for other getStaticProps/getStaticPaths during the build.
| const [blogPostsWithRelated, stats] = await Promise.all([fetchBlogPosts(), fetchAnalyticsSummary()]); | |
| // omitBlogPosts was unused by LandingPage, so we drop it here | |
| void blogPostsWithRelated; | |
| const stats = await fetchAnalyticsSummary(); |
Restore the functionality to fetch and display analytics summary data on the landing page. This includes integrating the analytics fetching logic and updating the landing page component to utilize the fetched data.