Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ env:
# we don't want to let the PR be merged yet. If the "Staging API" label is added to the PR, we
# run codegen against the staging API instead of the production API and block the PR from merging.
API_URL: ${{ contains(github.event.pull_request.labels.*.name, 'Staging API') && 'https://api.stage.mpdx.org/graphql' || 'https://api.mpdx.org/graphql' }}
SITE_URL: http://stage.mpdx.org

jobs:
test:
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/preview.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,3 @@ jobs:
AmplifyAppId: ${{ secrets.AMPLIFY_APP_ID }}
AWS_REGION: 'us-east-1'
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
EnvironmentVariables: 'SITE_URL=https://${{ github.head_ref }}.${{ secrets.AMPLIFY_APP_ID }}.amplifyapp.com'
1 change: 0 additions & 1 deletion __tests__/util/globalSetup.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
const globalSetup = (): void => {
process.env.TZ = 'UTC';
process.env.JWT_SECRET = 'test-environment-key';
process.env.SITE_URL = 'https://mpdx.org';
process.env.APP_NAME = 'MPDX';
};

Expand Down
7 changes: 2 additions & 5 deletions next.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ if (prod && !process.env.JWT_SECRET) {
throw new Error('JWT_SECRET environment variable is not set');
}

const siteUrl =
process.env.PREVIEW_URL ?? process.env.SITE_URL ?? 'http://localhost:3000';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can also update the amplify.yml as we no longer need the PREVIEW_URL

Copy link
Copy Markdown
Contributor Author

@zweatshirt zweatshirt May 18, 2026

Choose a reason for hiding this comment

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

@dr-bizz Is it still needed for NEXTAUTH_URL=$PREVIEW_URL; in amplify.yml?
https://github.com/CruGlobal/mpdx-react/blob/site-url/amplify.yml#L16

We can remove echo "PREVIEW_URL=$PREVIEW_URL" >> .env; from amplify.yml
https://github.com/CruGlobal/mpdx-react/blob/site-url/amplify.yml#L17

Just not sure about line 16


const withBundleAnalyzer = bundleAnalyzer({
enabled: process.env.ANALYZE === 'true',
});
Expand All @@ -38,14 +35,14 @@ const config: NextConfig = {
REST_API_URL:
process.env.REST_API_URL ?? 'https://api.stage.mpdx.org/api/v2/',
OAUTH_URL: process.env.OAUTH_URL ?? 'https://auth.stage.mpdx.org',
SITE_URL: siteUrl,
CLIENT_ID: process.env.CLIENT_ID ?? '4027334344069527005',
CLIENT_SECRET: process.env.CLIENT_SECRET,
AUTH_PROVIDER: process.env.AUTH_PROVIDER ?? 'OKTA',
OKTA_CLIENT_ID: process.env.OKTA_CLIENT_ID ?? '0oa1n0gjoy3j5Ycdg0h8',
OKTA_CLIENT_SECRET: process.env.OKTA_CLIENT_SECRET,
OKTA_ISSUER: process.env.OKTA_ISSUER ?? 'https://signon.okta.com',
OKTA_SIGNOUT_REDIRECT_URL: process.env.OKTA_SIGNOUT_REDIRECT_URL ?? siteUrl,
OKTA_SIGNOUT_REDIRECT_URL:
process.env.OKTA_SIGNOUT_REDIRECT_URL ?? 'http://localhost:3000',
API_OAUTH_CLIENT_ID:
process.env.API_OAUTH_CLIENT_ID ??
'3nxoth_gyetHdpjKp2WYkND1PUQlvYcjXQHW9ZdDxq4',
Expand Down
2 changes: 1 addition & 1 deletion pages/_document.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
{process.env.HELPJUICE_ORIGIN && (
<script
dangerouslySetInnerHTML={{
__html: `window.helpjuice_account_url = '${process.env.SITE_URL}/api/helpjuice';
__html: `window.helpjuice_account_url = window.location.origin + '/api/helpjuice';
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Suggestion] **Inline-script pattern is correct; informational only.**

The new line emits the literal text window.location.origin + '/api/helpjuice' verbatim into a <script> tag, which the browser evaluates at script-execution time. Strictly safer than the previous ${process.env.SITE_URL} interpolation — no env-var content is being injected into a JS string literal anymore. window.location.origin is a browser-trusted DOM value (not user input) and is available synchronously in all supported browsers.

Mixed pattern with ${process.env.HELPJUICE_ORIGIN} on the next line (build-time inline) is intentional and principled: app's own origin is variable across previews → resolved at runtime; third-party origins are stable per environment → inlined at build. A one-line comment near this script noting that split would help future readers, but no change required.

window.helpjuice_contact_us_url = '${process.env.HELPJUICE_ORIGIN}/contact-us';
window.helpjuiceSwiftyConfig = { widgetPosition: 'bottomRight' };
window.helpjuiceSwiftyUrlMap = {};`,
Expand All @@ -50,7 +50,7 @@
/>
)}
{process.env.HELPJUICE_ORIGIN && (
<script src={`${process.env.HELPJUICE_ORIGIN}/swifty.js`} />

Check warning on line 53 in pages/_document.page.tsx

View workflow job for this annotation

GitHub Actions / eslint

Synchronous scripts should not be used. See: https://nextjs.org/docs/messages/no-sync-scripts

Check warning on line 53 in pages/_document.page.tsx

View workflow job for this annotation

GitHub Actions / eslint

Synchronous scripts should not be used. See: https://nextjs.org/docs/messages/no-sync-scripts
)}
{process.env.DATADOG_CONFIGURED === 'true' && (
<Script id="datadog-rum" strategy="afterInteractive">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,11 +223,8 @@ describe('ProfileMenu', () => {
describe('ProfileMenu while Impersonating', () => {
fetchMock.enableMocks();
window = Object.create(window);
const url = 'https://mpdx.org';
Object.defineProperty(window, 'location', {
value: {
href: url,
},
value: {},
writable: true,
});

Expand Down Expand Up @@ -275,9 +272,7 @@ describe('ProfileMenu while Impersonating', () => {
);

await waitFor(() =>
expect(window.location.href).toEqual(
`${process.env.SITE_URL}/api/stop-impersonating`,
),
expect(window.location.href).toEqual('/api/stop-impersonating'),
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ const ProfileMenu = (): ReactElement => {
},
);
window.localStorage.clear();
window.location.href = `${process.env.SITE_URL || window.location.origin}/api/stop-impersonating`;
window.location.href = '/api/stop-impersonating';
};

const handleAccountListClick = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ describe('GoogleAccordion', () => {

expect(getByText(/continue/i)).toHaveAttribute(
'href',
`https://auth.mpdx.org/auth/user/google?account_list_id=account-list-1&redirect_to=https%3A%2F%2Fmpdx.org%2FaccountLists%2Faccount-list-1%2Fsettings%2Fintegrations%3FselectedTab%3Dgoogle&access_token=apiToken`,
`https://auth.mpdx.org/auth/user/google?account_list_id=account-list-1&redirect_to=${encodeURIComponent(
`${window.location.origin}/accountLists/account-list-1/settings/integrations?selectedTab=google`,
)}&access_token=apiToken`,
);
});
});
Expand Down Expand Up @@ -224,7 +226,9 @@ describe('GoogleAccordion', () => {

expect(getByText(/continue/i)).toHaveAttribute(
'href',
`https://auth.mpdx.org/auth/user/google?account_list_id=account-list-1&redirect_to=https%3A%2F%2Fmpdx.org%2FaccountLists%2Faccount-list-1%2Fsettings%2Fintegrations%3FselectedTab%3Dgoogle&access_token=apiToken`,
`https://auth.mpdx.org/auth/user/google?account_list_id=account-list-1&redirect_to=${encodeURIComponent(
`${window.location.origin}/accountLists/account-list-1/settings/integrations?selectedTab=google`,
)}&access_token=apiToken`,
);
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ThemeProvider } from '@mui/material/styles';

Check notice on line 1 in src/components/Settings/integrations/Mailchimp/MailchimpAccordion.test.tsx

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ No longer an issue: Lines of Code in a Single File

The lines of code in this module is no longer above the threshold
import { act, render, waitFor, within } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { SnackbarProvider } from 'notistack';
Expand Down Expand Up @@ -139,7 +139,9 @@

expect(getByText('Connect Mailchimp')).toHaveAttribute(
'href',
`https://auth.mpdx.org/auth/user/mailchimp?account_list_id=account-list-1&redirect_to=https%3A%2F%2Fmpdx.org%2FaccountLists%2Faccount-list-1%2Fsettings%2Fintegrations%3FselectedTab%3Dmailchimp&access_token=apiToken`,
`https://auth.mpdx.org/auth/user/mailchimp?account_list_id=account-list-1&redirect_to=${encodeURIComponent(
`${window.location.origin}/accountLists/account-list-1/settings/integrations?selectedTab=mailchimp`,
)}&access_token=apiToken`,
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,9 @@ describe('OrganizationAccordion', () => {
});

expect(window.location.assign).toHaveBeenCalledWith(
'https://auth.mpdx.org/auth/user/donorhub?account_list_id=account-list-1&redirect_to=https%3A%2F%2Fmpdx.org%2FaccountLists%2Faccount-list-1%2Fsettings%2Fintegrations%3FselectedTab%3Dorganization&access_token=apiToken&organization_id=organizationId',
`https://auth.mpdx.org/auth/user/donorhub?account_list_id=account-list-1&redirect_to=${encodeURIComponent(
`${window.location.origin}/accountLists/account-list-1/settings/integrations?selectedTab=organization`,
)}&access_token=apiToken&organization_id=organizationId`,
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ describe('PrayerlettersAccount', () => {

expect(getByText('Connect prayerletters.com Account')).toHaveAttribute(
'href',
`https://auth.mpdx.org/auth/user/prayer_letters?account_list_id=account-list-1&redirect_to=https%3A%2F%2Fmpdx.org%2FaccountLists%2Faccount-list-1%2Fsettings%2Fintegrations%3FselectedTab%3Dprayerletters.com&access_token=apiToken`,
`https://auth.mpdx.org/auth/user/prayer_letters?account_list_id=account-list-1&redirect_to=${encodeURIComponent(
`${window.location.origin}/accountLists/account-list-1/settings/integrations?selectedTab=prayerletters.com`,
)}&access_token=apiToken`,
);
});
});
Expand Down Expand Up @@ -153,7 +155,9 @@ describe('PrayerlettersAccount', () => {

expect(getByText('Refresh prayerletters.com Account')).toHaveAttribute(
'href',
`https://auth.mpdx.org/auth/user/prayer_letters?account_list_id=account-list-1&redirect_to=https%3A%2F%2Fmpdx.org%2FaccountLists%2Faccount-list-1%2Fsettings%2Fintegrations%3FselectedTab%3Dprayerletters.com&access_token=apiToken`,
`https://auth.mpdx.org/auth/user/prayer_letters?account_list_id=account-list-1&redirect_to=${encodeURIComponent(
`${window.location.origin}/accountLists/account-list-1/settings/integrations?selectedTab=prayerletters.com`,
)}&access_token=apiToken`,
);

userEvent.click(
Expand Down
2 changes: 1 addition & 1 deletion src/components/Settings/integrations/useOauthUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export const useOauthUrl = () => {
const accountListId = useAccountListId();

const getRedirectUrl = (accordion: IntegrationAccordion) => {
const domain = process.env.SITE_URL || window.location.origin;
const domain = window.location.origin;
return encodeURIComponent(
`${domain}/accountLists/${accountListId}/settings/integrations?selectedTab=${accordion}`,
);
Expand Down
35 changes: 35 additions & 0 deletions src/components/Tool/MergeContacts/MergeContacts.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -260,4 +260,39 @@ describe('Tools - MergeContacts', () => {
`/accountLists/${accountListId}/tools/merge/contacts/contact-2`,
);
});

it('should redirect to tools after Confirm and Leave', async () => {
Object.defineProperty(window, 'location', {
value: {},
writable: true,
});

const { queryAllByTestId, getByRole } = render(
<SnackbarProvider>
<MergeContactsWrapper
mocks={{
GetContactDuplicates:
getContactDuplicatesMocks.GetContactDuplicates,
MassActionsMerge: () => {
return { mergeContacts: ['contact-2'] };
},
}}
/>
</SnackbarProvider>,
);

await waitFor(() =>
expect(queryAllByTestId('MergeContactPair')).toHaveLength(2),
);
userEvent.click(queryAllByTestId('ignoreButton')[0]);
userEvent.click(queryAllByTestId('ignoreButton')[1]);

userEvent.click(getByRole('button', { name: 'Confirm and Leave' }));

await waitFor(() =>
expect(window.location.href).toEqual(
`/accountLists/${accountListId}/tools`,
),
);
});
});
2 changes: 1 addition & 1 deletion src/components/Tool/MergeContacts/StickyConfirmButtons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const StickyConfirmButtons: React.FC<StickyConfirmButtonsProps> = ({
const handleConfirmAndLeave = async () => {
await confirmAction();
setActions({});
window.location.href = `${process.env.SITE_URL}/accountLists/${accountListId}/tools`;
window.location.href = `/accountLists/${accountListId}/tools`;
};
return (
<StickyButtonHeaderBox>
Expand Down
8 changes: 8 additions & 0 deletions src/components/Tool/TntConnect/TntConnect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ describe('TntConnect Import', () => {
});

it('should handle uploading a file', async () => {
Object.defineProperty(window, 'location', {
value: {},
writable: true,
});

const {
getByRole,
queryByText,
Expand Down Expand Up @@ -88,6 +93,9 @@ describe('TntConnect Import', () => {
await waitFor(() =>
expect(queryByText('Good Work!')).not.toBeInTheDocument(),
);
expect(window.location.href).toEqual(
`/accountLists/${accountListId}/tools`,
);

await waitFor(() =>
expect(uploadTnt).toHaveBeenCalledWith(
Expand Down
2 changes: 1 addition & 1 deletion src/components/Tool/TntConnect/TntConnect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ const TntConnect: React.FC<Props> = ({ accountListId }: Props) => {

const handleCloseModal = () => {
setShowModal(false);
window.location.href = `${process.env.SITE_URL}/accountLists/${accountListId}/tools`;
window.location.href = `/accountLists/${accountListId}/tools`;
setLoading(true);
};

Expand Down
2 changes: 1 addition & 1 deletion src/lib/apollo/link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const batchNativeHttpLink = new BatchHttpLink({
});

const restProxyHttpLink = new BatchHttpLink({
uri: `${process.env.SITE_URL}/api/graphql-rest`,
uri: `/api/graphql-rest`,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Suggestion] **Latent SSR safety risk.** This module is imported by both `client.ts` (browser) and `ssrClient.ts` (Node SSR). The relative URI `/api/graphql-rest` resolves correctly via `window.location.origin` in the browser, but Node's `fetch` rejects relative URLs.

Today no SSR query routes here — verified against src/graphql/rootFields.generated.ts: all current SSR queries (GetAccountLists, AccountListOptions, UpdateUserDefaultAccount, GetDashboard, GetDefaultAccount) select only native root fields, so restProxyHttpLink is never invoked from the server. No current bug.

The risk is future-only: a developer who adds a REST-proxy-only field to an SSR query will get a confusing TypeError: Failed to parse URL from /api/graphql-rest at runtime in production. Optional defensive fix:

const restProxyHttpLink = new BatchHttpLink({
  uri: () => {
    if (typeof window === 'undefined') {
      throw new Error(
        'REST-proxy operations cannot run from SSR — add the field to the native GraphQL API or move the query to a client-side hook.',
      );
    }
    return '/api/graphql-rest';
  },
  batchMax: 25,
  batchDebounce: true,
  batchInterval: 20,
});

Not a blocker — informational only.

batchMax: 25,
batchDebounce: true,
batchInterval: 20,
Expand Down
Loading