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

fix: Init analytics context from API data #504

Merged
merged 3 commits into from
Feb 7, 2022
Merged

Conversation

suzubara
Copy link
Contributor

@suzubara suzubara commented Feb 2, 2022

Description

This PR changes how the AnalyticsContext is initialized. Instead of using NextJS runtime variables (which produced inconsistent & confusing results), the analytics code makes a request to /api/sysinfo to query the Matomo URL & site ID in the browser. This lets us set those environment variables at runtime without having to worry about build variables. We may revisit this in the future if we start building images per-environment, but for now it should make the analytics init more consistent.

Fixes #501

Review Notes

  • Log in to the portal and navigate to the beta landing page and/or the Sites & Applications page. Open the browser JS console and refresh the page. You should not see the error ANALYTICS: No Matomo URL provided.

@@ -10,5 +10,7 @@ export default function handler(req: NextApiRequest, res: NextApiResponse) {
version: __VERSION__,
buildId: __BUILD_ID__,
nodeEnv: __NODE_ENV__,
analyticsUrl: process.env.MATOMO_URL,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abbyoung this info seemed appropriate to add to this existing endpoint, but let me know if you feel otherwise!!

@suzubara suzubara merged commit d46185f into main Feb 7, 2022
@suzubara suzubara deleted the 501-analytics-config-env branch February 7, 2022 18:56
@suzubara suzubara mentioned this pull request Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Analytics config is not being loaded into NextJS from environment
2 participants