Skip to content

Point user-portal links at the configured portal URL - #719

Open
johnworth wants to merge 1 commit into
mainfrom
fix-user-portal-url
Open

Point user-portal links at the configured portal URL#719
johnworth wants to merge 1 commit into
mainfrom
fix-user-portal-url

Conversation

@johnworth

Copy link
Copy Markdown
Contributor

The logged-out banner kept the signup link in a useRef seeded with constants.DEFAULT_USER_PORTAL_URL, updated it from a useEffect, and then read the ref during render. Mutating a ref triggers no re-render, so the link stayed on https://user.cyverse.org until some unrelated re-render happened to occur -- and every refresh reverted it, since the SSR'd HTML always carried the default.

Read publicRuntimeConfig.USER_PORTAL_URL directly instead of going through the config context: the context is assembled in an effect in _app, so it is still null when the banner first renders and would leave the initial paint pointing at the default either way.

The register link on the sign-in error page and the user-portal link in the admin subscription drawer had the same problem in a simpler form -- they read the hardcoded constants and never consulted the config at all. That leaves constants.USER_PORTAL unused, so drop it.

The logged-out banner kept the signup link in a useRef seeded with
constants.DEFAULT_USER_PORTAL_URL, updated it from a useEffect, and then
read the ref during render. Mutating a ref triggers no re-render, so the
link stayed on https://user.cyverse.org until some unrelated re-render
happened to occur -- and every refresh reverted it, since the SSR'd HTML
always carried the default.

Read publicRuntimeConfig.USER_PORTAL_URL directly instead of going
through the config context: the context is assembled in an effect in
_app, so it is still null when the banner first renders and would leave
the initial paint pointing at the default either way.

The register link on the sign-in error page and the user-portal link in
the admin subscription drawer had the same problem in a simpler form --
they read the hardcoded constants and never consulted the config at all.
That leaves constants.USER_PORTAL unused, so drop it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@psarando psarando left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM 👍

@psarando psarando left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wait, I'll have to change my review, since this will cause conflicts in the next.js v16 upgrade, which gets rid of the publicRuntimeConfig.

It can be kept in the Banner for this PR, and I'll do some testing in the nextjs v16 branch for another solution, if the useConfig context won't work there.

Comment on lines +64 to +67
const { publicRuntimeConfig = {} } = getConfig() || {};
const registerURL = `${
publicRuntimeConfig.USER_PORTAL_URL || constants.DEFAULT_USER_PORTAL_URL
}/register`;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can this use the useConfig context instead of the publicRuntimeConfig?

Comment on lines +296 to +299
const { publicRuntimeConfig = {} } = getConfig() || {};
const baseURL =
publicRuntimeConfig.USER_PORTAL_URL ||
constants.DEFAULT_USER_PORTAL_URL;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can this use the useConfig context instead of the publicRuntimeConfig?

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.

2 participants