From 17ca8f880133e0fa781ad046786eaaa736324983 Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Wed, 6 Mar 2019 11:11:41 -0500 Subject: [PATCH 01/20] Factor out createLetterOfComplaintRouteInfo(). --- frontend/lib/routes.ts | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/frontend/lib/routes.ts b/frontend/lib/routes.ts index bf2555438..87b32f8b5 100644 --- a/frontend/lib/routes.ts +++ b/frontend/lib/routes.ts @@ -87,6 +87,22 @@ function createOnboardingRouteInfo(prefix: string) { }; } +export type LetterOfComplaintInfo = ReturnType; + +function createLetterOfComplaintRouteInfo(prefix: string) { + return { + [ROUTE_PREFIX]: prefix, + latestStep: prefix, + home: `${prefix}/welcome`, + issues: createIssuesRouteInfo(`${prefix}/issues`), + accessDates: `${prefix}/access-dates`, + yourLandlord: `${prefix}/your-landlord`, + preview: `${prefix}/preview`, + previewSendConfirmModal: `${prefix}/preview/send-confirm-modal`, + confirmation: `${prefix}/confirmation` + }; +} + /** * This is an ad-hoc structure that defines URL routes for our app. */ @@ -111,17 +127,7 @@ const Routes = { onboarding: createOnboardingRouteInfo('/onboarding'), /** The Letter of Complaint flow. */ - loc: { - [ROUTE_PREFIX]: '/loc', - latestStep: '/loc', - home: '/loc/welcome', - issues: createIssuesRouteInfo('/loc/issues'), - accessDates: '/loc/access-dates', - yourLandlord: '/loc/your-landlord', - preview: '/loc/preview', - previewSendConfirmModal: '/loc/preview/send-confirm-modal', - confirmation: '/loc/confirmation' - }, + loc: createLetterOfComplaintRouteInfo('/loc'), /** The HP Action flow. */ hp: { From f79e7886b5092472425c2ea704fc8fd1f9701491 Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Wed, 6 Mar 2019 11:17:31 -0500 Subject: [PATCH 02/20] Factor out createHPActionRouteInfo(). --- frontend/lib/routes.ts | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/frontend/lib/routes.ts b/frontend/lib/routes.ts index 87b32f8b5..a3133aa70 100644 --- a/frontend/lib/routes.ts +++ b/frontend/lib/routes.ts @@ -103,6 +103,24 @@ function createLetterOfComplaintRouteInfo(prefix: string) { }; } +export type HPActionInfo = ReturnType; + +function createHPActionRouteInfo(prefix: string) { + return { + [ROUTE_PREFIX]: prefix, + latestStep: prefix, + preOnboarding: `${prefix}/splash`, + splash: `${prefix}/splash`, + onboarding: createOnboardingRouteInfo(`${prefix}/onboarding`), + postOnboarding: prefix, + welcome: `${prefix}/welcome`, + issues: createIssuesRouteInfo(`${prefix}/issues`), + yourLandlord: `${prefix}/your-landlord`, + waitForUpload: `${prefix}/wait`, + confirmation: `${prefix}/confirmation`, + } +} + /** * This is an ad-hoc structure that defines URL routes for our app. */ @@ -130,19 +148,7 @@ const Routes = { loc: createLetterOfComplaintRouteInfo('/loc'), /** The HP Action flow. */ - hp: { - [ROUTE_PREFIX]: '/hp', - latestStep: '/hp', - preOnboarding: '/hp/splash', - splash: '/hp/splash', - onboarding: createOnboardingRouteInfo('/hp/onboarding'), - postOnboarding: '/hp', - welcome: '/hp/welcome', - issues: createIssuesRouteInfo('/hp/issues'), - yourLandlord: '/hp/your-landlord', - waitForUpload: '/hp/wait', - confirmation: '/hp/confirmation', - }, + hp: createHPActionRouteInfo('/hp'), /** * Example pages used in integration tests, and other From ec39d29695fcada7f83469b5c82f386a6a88c227 Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Wed, 6 Mar 2019 11:42:04 -0500 Subject: [PATCH 03/20] Factor out createLocalizedRouteInfo(). --- frontend/lib/routes.ts | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/frontend/lib/routes.ts b/frontend/lib/routes.ts index a3133aa70..b8e864ff1 100644 --- a/frontend/lib/routes.ts +++ b/frontend/lib/routes.ts @@ -121,12 +121,35 @@ function createHPActionRouteInfo(prefix: string) { } } +export type LocalizedRouteInfo = ReturnType; + +function createLocalizedRouteInfo(prefix: string) { + return { + /** The login page. */ + login: `${prefix}/login`, + + /** The logout page. */ + logout: `${prefix}/logout`, + + /** The home page. */ + home: `${prefix}/`, + + /** The onboarding flow. */ + onboarding: createOnboardingRouteInfo(`${prefix}/onboarding`), + + /** The Letter of Complaint flow. */ + loc: createLetterOfComplaintRouteInfo(`${prefix}/loc`), + + /** The HP Action flow. */ + hp: createHPActionRouteInfo(`${prefix}/hp`), + } +} + /** * This is an ad-hoc structure that defines URL routes for our app. */ const Routes = { - /** The login page. */ - login: '/login', + ...createLocalizedRouteInfo(''), /** * The *admin* login page. We override Django's default admin login @@ -135,21 +158,6 @@ const Routes = { */ adminLogin: '/admin/login/', - /** The logout page. */ - logout: '/logout', - - /** The home page. */ - home: '/', - - /** The onboarding flow. */ - onboarding: createOnboardingRouteInfo('/onboarding'), - - /** The Letter of Complaint flow. */ - loc: createLetterOfComplaintRouteInfo('/loc'), - - /** The HP Action flow. */ - hp: createHPActionRouteInfo('/hp'), - /** * Example pages used in integration tests, and other * development-related pages. From dc1276e4ab9df73a4404b3df57a261cb69f3aa63 Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Wed, 6 Mar 2019 12:11:01 -0500 Subject: [PATCH 04/20] Add Routes.locale. --- frontend/lib/app.tsx | 10 +++--- frontend/lib/hp-action.tsx | 32 +++++++++---------- frontend/lib/letter-of-complaint.tsx | 22 ++++++------- frontend/lib/navbar.tsx | 6 ++-- frontend/lib/pages/access-dates.tsx | 4 +-- frontend/lib/pages/example-form-page.tsx | 2 +- frontend/lib/pages/index-page.tsx | 4 +-- frontend/lib/pages/landlord-details.tsx | 4 +-- frontend/lib/pages/letter-request.tsx | 6 ++-- frontend/lib/pages/login-page.tsx | 2 +- frontend/lib/pages/logout-page.tsx | 4 +-- frontend/lib/routes.ts | 11 ++++--- frontend/lib/tests/hp-action.test.tsx | 8 ++--- .../lib/tests/letter-of-complaint.test.tsx | 4 +-- frontend/lib/tests/onboarding.test.tsx | 12 +++---- .../lib/tests/pages/access-dates.test.tsx | 2 +- frontend/lib/tests/pages/index-page.test.tsx | 2 +- frontend/lib/tests/pages/issue-pages.test.tsx | 4 +-- .../lib/tests/pages/landlord-details.test.tsx | 6 ++-- .../lib/tests/pages/letter-request.test.tsx | 4 +-- .../lib/tests/pages/loc-confirmation.test.tsx | 2 +- .../tests/pages/onboarding-step-1.test.tsx | 2 +- .../tests/pages/onboarding-step-2.test.tsx | 2 +- .../tests/pages/onboarding-step-3.test.tsx | 2 +- .../tests/pages/onboarding-step-4.test.tsx | 2 +- 25 files changed, 80 insertions(+), 79 deletions(-) diff --git a/frontend/lib/app.tsx b/frontend/lib/app.tsx index 130c026e4..7e04a9d8d 100644 --- a/frontend/lib/app.tsx +++ b/frontend/lib/app.tsx @@ -189,16 +189,16 @@ export class AppWithoutRouter extends React.Component): JSX.Element { return ( - + - + - + {getOnboardingRouteForIntent(OnboardingInfoSignupIntent.LOC)} - + {getOnboardingRouteForIntent(OnboardingInfoSignupIntent.HP)} - + diff --git a/frontend/lib/hp-action.tsx b/frontend/lib/hp-action.tsx index 729741dab..cc1f8b8da 100644 --- a/frontend/lib/hp-action.tsx +++ b/frontend/lib/hp-action.tsx @@ -16,7 +16,7 @@ import { GetHPActionUploadStatus } from './queries/GetHPActionUploadStatus'; import { Redirect } from 'react-router'; import { SessionPoller } from './session-poller'; -const onboardingForHPActionRoute = Routes.hp.onboarding.latestStep; +const onboardingForHPActionRoute = Routes.locale.hp.onboarding.latestStep; function HPActionSplash(): JSX.Element { return ( @@ -48,7 +48,7 @@ const HPActionWelcome = withAppContext((props: AppContextType) => {
  • Print out this packet and bring it to Housing Court. It will include instructions for filing in court and serving your landlord.
    • - + Select repair issues
    @@ -62,9 +62,9 @@ const HPActionWelcome = withAppContext((props: AppContextType) => { const HPActionIssuesRoutes = () => ( ); @@ -83,7 +83,7 @@ const LandlordDetails = (props: { details: AllSessionInfo_landlordDetails }) => const GeneratePDFForm = (props: { children: FormContextRenderer<{}> }) => ( + onSuccessRedirect={Routes.locale.hp.waitForUpload} {...props} /> ); const HPActionYourLandlord = withAppContext((props: AppContextType) => { @@ -98,7 +98,7 @@ const HPActionYourLandlord = withAppContext((props: AppContextType) => { {(ctx) =>
    - +
    } @@ -139,13 +139,13 @@ const ShowHPUploadStatus = withAppContext((props: AppContextType) => { return ; case HPUploadStatus.SUCCEEDED: - return ; + return ; case HPUploadStatus.ERRORED: return ; case HPUploadStatus.NOT_STARTED: - return ; + return ; } }); @@ -175,23 +175,23 @@ const HPActionConfirmation = withAppContext((props: AppContextType) => { }); export const HPActionProgressRoutesProps: ProgressRoutesProps = { - toLatestStep: Routes.hp.latestStep, + toLatestStep: Routes.locale.hp.latestStep, label: "HP Action", welcomeSteps: [{ - path: Routes.hp.splash, exact: true, component: HPActionSplash, + path: Routes.locale.hp.splash, exact: true, component: HPActionSplash, isComplete: (s) => !!s.phoneNumber }, { - path: Routes.hp.welcome, exact: true, component: HPActionWelcome + path: Routes.locale.hp.welcome, exact: true, component: HPActionWelcome }], stepsToFillOut: [ - { path: Routes.hp.issues.prefix, component: HPActionIssuesRoutes }, - { path: Routes.hp.yourLandlord, exact: true, component: HPActionYourLandlord, + { path: Routes.locale.hp.issues.prefix, component: HPActionIssuesRoutes }, + { path: Routes.locale.hp.yourLandlord, exact: true, component: HPActionYourLandlord, isComplete: (s) => s.hpActionUploadStatus !== HPUploadStatus.NOT_STARTED }, ], confirmationSteps: [ - { path: Routes.hp.waitForUpload, exact: true, component: ShowHPUploadStatus, + { path: Routes.locale.hp.waitForUpload, exact: true, component: ShowHPUploadStatus, isComplete: (s) => s.hpActionUploadStatus === HPUploadStatus.SUCCEEDED }, - { path: Routes.hp.confirmation, exact: true, component: HPActionConfirmation} + { path: Routes.locale.hp.confirmation, exact: true, component: HPActionConfirmation} ] }; diff --git a/frontend/lib/letter-of-complaint.tsx b/frontend/lib/letter-of-complaint.tsx index 10c24337b..06a9148b1 100644 --- a/frontend/lib/letter-of-complaint.tsx +++ b/frontend/lib/letter-of-complaint.tsx @@ -24,7 +24,7 @@ export const Welcome = withAppContext((props: AppContextType): JSX.Element => {
  • First, conduct a self-inspection of your apartment to document all the issues that need repair.
  • Review your Letter of Complaint and JustFix.nyc will send it to your landlord via USPS Certified Mail®.
    • - + Start my free letter
    @@ -42,27 +42,27 @@ export const Welcome = withAppContext((props: AppContextType): JSX.Element => { const LetterOfComplaintIssuesRoutes = () => ( ); export const LOCProgressRoutesProps: ProgressRoutesProps = { - toLatestStep: Routes.loc.latestStep, + toLatestStep: Routes.locale.loc.latestStep, label: "Letter of Complaint", welcomeSteps: [{ - path: Routes.loc.home, exact: true, component: Welcome + path: Routes.locale.loc.home, exact: true, component: Welcome }], stepsToFillOut: [ - { path: Routes.loc.issues.prefix, component: LetterOfComplaintIssuesRoutes }, - { path: Routes.loc.accessDates, exact: true, component: AccessDatesPage }, - { path: Routes.loc.yourLandlord, exact: true, component: LandlordDetailsPage }, - { path: Routes.loc.preview, component: LetterRequestPage, + { path: Routes.locale.loc.issues.prefix, component: LetterOfComplaintIssuesRoutes }, + { path: Routes.locale.loc.accessDates, exact: true, component: AccessDatesPage }, + { path: Routes.locale.loc.yourLandlord, exact: true, component: LandlordDetailsPage }, + { path: Routes.locale.loc.preview, component: LetterRequestPage, isComplete: sess => !!sess.letterRequest }, ], confirmationSteps: [{ - path: Routes.loc.confirmation, exact: true, component: LetterConfirmation + path: Routes.locale.loc.confirmation, exact: true, component: LetterConfirmation }] }; diff --git a/frontend/lib/navbar.tsx b/frontend/lib/navbar.tsx index 0a5498fde..1d8c7587e 100644 --- a/frontend/lib/navbar.tsx +++ b/frontend/lib/navbar.tsx @@ -118,7 +118,7 @@ class NavbarWithoutAppContext extends React.Component return (
    - +
    {session.isStaff && Admin} {session.phoneNumber - ? Sign out - : Sign in } + ? Sign out + : Sign in } {this.renderDevMenu()}
    diff --git a/frontend/lib/pages/access-dates.tsx b/frontend/lib/pages/access-dates.tsx index 9ec53a555..8e39a5709 100644 --- a/frontend/lib/pages/access-dates.tsx +++ b/frontend/lib/pages/access-dates.tsx @@ -36,7 +36,7 @@ function renderForm(ctx: FormContext): JSX.Element {
    - +
    @@ -52,7 +52,7 @@ export default function AccessDatesPage(): JSX.Element { getInitialState(session.accessDates)} - onSuccessRedirect={Routes.loc.yourLandlord} + onSuccessRedirect={Routes.locale.loc.yourLandlord} > {renderForm} diff --git a/frontend/lib/pages/example-form-page.tsx b/frontend/lib/pages/example-form-page.tsx index 8879e9810..39f8d2281 100644 --- a/frontend/lib/pages/example-form-page.tsx +++ b/frontend/lib/pages/example-form-page.tsx @@ -71,7 +71,7 @@ export default function ExampleFormPage(): JSX.Element { Use the form in a modal - + ); } diff --git a/frontend/lib/pages/index-page.tsx b/frontend/lib/pages/index-page.tsx index 7b7e34aba..2ae4f5ca2 100644 --- a/frontend/lib/pages/index-page.tsx +++ b/frontend/lib/pages/index-page.tsx @@ -13,7 +13,7 @@ export interface IndexPageProps { isLoggedIn: boolean; } -const onboardingForLOCRoute = Routes.onboarding.latestStep; +const onboardingForLOCRoute = Routes.locale.onboarding.latestStep; export default class IndexPage extends React.Component { renderLoggedOut() { @@ -35,7 +35,7 @@ export default class IndexPage extends React.Component { Start my free letter -

    Already have an account? Sign in!

    +

    Already have an account? Sign in!

    diff --git a/frontend/lib/pages/landlord-details.tsx b/frontend/lib/pages/landlord-details.tsx index fce34aa8a..7b987c885 100644 --- a/frontend/lib/pages/landlord-details.tsx +++ b/frontend/lib/pages/landlord-details.tsx @@ -19,9 +19,9 @@ const BLANK_INPUT: LandlordDetailsInput = { address: '' }; -const PREV_STEP = Routes.loc.accessDates; +const PREV_STEP = Routes.locale.loc.accessDates; -const NEXT_STEP = Routes.loc.preview; +const NEXT_STEP = Routes.locale.loc.preview; function renderForm(ctx: FormContext): JSX.Element { return ( diff --git a/frontend/lib/pages/letter-request.tsx b/frontend/lib/pages/letter-request.tsx index 1308d4999..a49c7c6a2 100644 --- a/frontend/lib/pages/letter-request.tsx +++ b/frontend/lib/pages/letter-request.tsx @@ -56,7 +56,7 @@ function FormAsButton(props: FormAsButtonProps): JSX.Element { mutation={LetterRequestMutation} formId={'button_' + props.mailChoice} initialState={input} - onSuccessRedirect={Routes.loc.confirmation} + onSuccessRedirect={Routes.locale.loc.confirmation} > {(ctx) => <> @@ -79,11 +79,11 @@ export default function LetterRequestPage(): JSX.Element {

    Here is a preview of the letter for you to review. It includes the repair issues you selected from the Issue Checklist.

    - + Looks good to me!
    - + & AppContextType): JSX.Element => { let next = absolutifyURLToOurOrigin( - getPostOrQuerystringVar(props, NEXT) || Routes.home, + getPostOrQuerystringVar(props, NEXT) || Routes.locale.home, props.server.originURL ); diff --git a/frontend/lib/pages/logout-page.tsx b/frontend/lib/pages/logout-page.tsx index 6337b8dbb..4dc31e63d 100644 --- a/frontend/lib/pages/logout-page.tsx +++ b/frontend/lib/pages/logout-page.tsx @@ -19,7 +19,7 @@ export const LogoutPage = withAppContext((props: AppContextType) => { mutation={LogoutMutation} initialState={{}} // This looks odd but it's required for legacy POST to work. - onSuccessRedirect={Routes.logout} + onSuccessRedirect={Routes.locale.logout} >{(ctx) => ( )} @@ -30,7 +30,7 @@ export const LogoutPage = withAppContext((props: AppContextType) => { return (

    You are now signed out.

    -

    Sign back in

    +

    Sign back in

     ); } diff --git a/frontend/lib/routes.ts b/frontend/lib/routes.ts index b8e864ff1..34c58b105 100644 --- a/frontend/lib/routes.ts +++ b/frontend/lib/routes.ts @@ -23,12 +23,12 @@ type SignupIntentOnboardingInfo = { export function getSignupIntentOnboardingInfo(intent: OnboardingInfoSignupIntent): SignupIntentOnboardingInfo { switch (intent) { case OnboardingInfoSignupIntent.LOC: return { - preOnboarding: Routes.home, - postOnboarding: Routes.loc.latestStep, - onboarding: Routes.onboarding + preOnboarding: Routes.locale.home, + postOnboarding: Routes.locale.loc.latestStep, + onboarding: Routes.locale.onboarding }; - case OnboardingInfoSignupIntent.HP: return Routes.hp; + case OnboardingInfoSignupIntent.HP: return Routes.locale.hp; } } @@ -149,7 +149,8 @@ function createLocalizedRouteInfo(prefix: string) { * This is an ad-hoc structure that defines URL routes for our app. */ const Routes = { - ...createLocalizedRouteInfo(''), + /** Localized routes for the user's currently-selected locale. */ + locale: createLocalizedRouteInfo(''), /** * The *admin* login page. We override Django's default admin login diff --git a/frontend/lib/tests/hp-action.test.tsx b/frontend/lib/tests/hp-action.test.tsx index ee197211a..c4ab3bfc2 100644 --- a/frontend/lib/tests/hp-action.test.tsx +++ b/frontend/lib/tests/hp-action.test.tsx @@ -57,23 +57,23 @@ describe('upload status page', () => { describe('latest step redirector', () => { it('returns splash page when user is not logged-in', () => { - expect(tester.getLatestStep()).toBe(Routes.hp.splash); + expect(tester.getLatestStep()).toBe(Routes.locale.hp.splash); }); it('returns welcome page when user is logged-in', () => { - expect(tester.getLatestStep({ phoneNumber: '123' })).toBe(Routes.hp.welcome); + expect(tester.getLatestStep({ phoneNumber: '123' })).toBe(Routes.locale.hp.welcome); }); it('returns wait page when user has started document assembly', () => { expect(tester.getLatestStep({ hpActionUploadStatus: HPUploadStatus.STARTED - })).toBe(Routes.hp.waitForUpload); + })).toBe(Routes.locale.hp.waitForUpload); }); it('returns confirmation page when user has generated a PDF', () => { expect(tester.getLatestStep({ latestHpActionPdfUrl: '/boop.pdf', hpActionUploadStatus: HPUploadStatus.SUCCEEDED - })).toBe(Routes.hp.confirmation); + })).toBe(Routes.locale.hp.confirmation); }); }); diff --git a/frontend/lib/tests/letter-of-complaint.test.tsx b/frontend/lib/tests/letter-of-complaint.test.tsx index 8d42202b1..86b0f556a 100644 --- a/frontend/lib/tests/letter-of-complaint.test.tsx +++ b/frontend/lib/tests/letter-of-complaint.test.tsx @@ -8,12 +8,12 @@ tester.defineSmokeTests(); describe('latest step redirector', () => { it('returns welcome page by default', () => { - expect(tester.getLatestStep()).toBe(Routes.loc.home); + expect(tester.getLatestStep()).toBe(Routes.locale.loc.home); }); it('returns confirmation page if letter request has been submitted', () => { expect(tester.getLatestStep({ letterRequest: {} as any - })).toBe(Routes.loc.confirmation); + })).toBe(Routes.locale.loc.confirmation); }); }); diff --git a/frontend/lib/tests/onboarding.test.tsx b/frontend/lib/tests/onboarding.test.tsx index 5a4110dc6..058451195 100644 --- a/frontend/lib/tests/onboarding.test.tsx +++ b/frontend/lib/tests/onboarding.test.tsx @@ -11,7 +11,7 @@ import { OnboardingInfoSignupIntent } from '../queries/globalTypes'; const PROPS: OnboardingRoutesProps = { toCancel: '/cancel', toSuccess: '/success', - routes: Routes.onboarding, + routes: Routes.locale.onboarding, signupIntent: OnboardingInfoSignupIntent.LOC }; @@ -22,14 +22,14 @@ describe('latest step redirector', () => { } it('returns step 1 by default', () => { - expect(getLatestOnboardingStep(FakeSessionInfo)).toBe(Routes.onboarding.step1); + expect(getLatestOnboardingStep(FakeSessionInfo)).toBe(Routes.locale.onboarding.step1); }); it('returns step 2 when step 1 is complete', () => { expect(getLatestOnboardingStep({ ...FakeSessionInfo, onboardingStep1: {} as any - })).toBe(Routes.onboarding.step2); + })).toBe(Routes.locale.onboarding.step2); }); it('returns step 3 when step 2 is complete', () => { @@ -37,7 +37,7 @@ describe('latest step redirector', () => { ...FakeSessionInfo, onboardingStep1: {} as any, onboardingStep2: {} as any - })).toBe(Routes.onboarding.step3); + })).toBe(Routes.locale.onboarding.step3); }); it('returns step 4 when step 3 is complete', () => { @@ -46,7 +46,7 @@ describe('latest step redirector', () => { onboardingStep1: {} as any, onboardingStep2: {} as any, onboardingStep3: {} as any - })).toBe(Routes.onboarding.step4); + })).toBe(Routes.locale.onboarding.step4); }); }); @@ -55,7 +55,7 @@ describe('Onboarding', () => { it('redirects to latest step', () => { const pal = new AppTesterPal(, { - url: Routes.onboarding.latestStep + url: Routes.locale.onboarding.latestStep }); expect(pal.history.location.pathname).toEqual('/onboarding/step/1'); pal.rr.getByLabelText('First name'); diff --git a/frontend/lib/tests/pages/access-dates.test.tsx b/frontend/lib/tests/pages/access-dates.test.tsx index 58c8c5fa6..9e41c7bb2 100644 --- a/frontend/lib/tests/pages/access-dates.test.tsx +++ b/frontend/lib/tests/pages/access-dates.test.tsx @@ -12,7 +12,7 @@ describe('access dates page', () => { it('redirects to next step after successful submission', async () => { const pal = new AppTesterPal(, { - url: Routes.loc.accessDates + url: Routes.locale.loc.accessDates }); pal.fillFormFields([ diff --git a/frontend/lib/tests/pages/index-page.test.tsx b/frontend/lib/tests/pages/index-page.test.tsx index a94ff11e9..222bbc0f8 100644 --- a/frontend/lib/tests/pages/index-page.test.tsx +++ b/frontend/lib/tests/pages/index-page.test.tsx @@ -5,7 +5,7 @@ import Routes from '../../routes'; describe('index page', () => { it('renders when logged in', () => { - ensureRedirect(, Routes.loc.latestStep); + ensureRedirect(, Routes.locale.loc.latestStep); }); it('renders when logged out', () => { diff --git a/frontend/lib/tests/pages/issue-pages.test.tsx b/frontend/lib/tests/pages/issue-pages.test.tsx index 6fb804f66..9e8375743 100644 --- a/frontend/lib/tests/pages/issue-pages.test.tsx +++ b/frontend/lib/tests/pages/issue-pages.test.tsx @@ -9,10 +9,10 @@ import ISSUE_AREA_SVGS from '../../svg/issues'; import { IssueAreaChoices } from '../../../../common-data/issue-area-choices'; -const routes = Routes.loc.issues; +const routes = Routes.locale.loc.issues; const TestIssuesRoutes = () => - ; + ; describe('issues checklist', () => { afterEach(AppTesterPal.cleanup); diff --git a/frontend/lib/tests/pages/landlord-details.test.tsx b/frontend/lib/tests/pages/landlord-details.test.tsx index abbc0e412..5611606b8 100644 --- a/frontend/lib/tests/pages/landlord-details.test.tsx +++ b/frontend/lib/tests/pages/landlord-details.test.tsx @@ -24,7 +24,7 @@ describe('landlord details page', () => { it('works when details are not looked up', () => { const pal = new AppTesterPal(, { - url: Routes.loc.yourLandlord, + url: Routes.locale.loc.yourLandlord, session: { landlordDetails: BLANK_LANDLORD_DETAILS } }); pal.rr.getByText(/Please enter your landlord's name/i); @@ -34,7 +34,7 @@ describe('landlord details page', () => { it('works when details are looked up', () => { const pal = new AppTesterPal(, { - url: Routes.loc.yourLandlord, + url: Routes.locale.loc.yourLandlord, session: { landlordDetails: LOOKED_UP_LANDLORD_DETAILS } }); pal.rr.getByText(/This may be different .+ rent checks/i); @@ -44,7 +44,7 @@ describe('landlord details page', () => { it('redirects to next step after successful submission', async () => { const pal = new AppTesterPal(, { - url: Routes.loc.yourLandlord, + url: Routes.locale.loc.yourLandlord, session: { landlordDetails: BLANK_LANDLORD_DETAILS } }); const name = "Boop Jones"; diff --git a/frontend/lib/tests/pages/letter-request.test.tsx b/frontend/lib/tests/pages/letter-request.test.tsx index 9e2bd3cf8..b7e00da84 100644 --- a/frontend/lib/tests/pages/letter-request.test.tsx +++ b/frontend/lib/tests/pages/letter-request.test.tsx @@ -31,7 +31,7 @@ describe('landlord details page', () => { it('works when user chooses to mail the letter themselves', async () => { const pal = new AppTesterPal(, { - url: Routes.loc.preview, + url: Routes.locale.loc.preview, session: { letterRequest: PRE_EXISTING_LETTER_REQUEST } }); clickButtonAndExpectChoice(pal, /mail this myself/i, LetterRequestMailChoice.USER_WILL_MAIL); @@ -39,7 +39,7 @@ describe('landlord details page', () => { it('works when user wants us to mail the letter', async () => { const pal = new AppTesterPal(, { - url: Routes.loc.preview, + url: Routes.locale.loc.preview, session: { letterRequest: PRE_EXISTING_LETTER_REQUEST } }); pal.clickButtonOrLink(/looks good to me/i); diff --git a/frontend/lib/tests/pages/loc-confirmation.test.tsx b/frontend/lib/tests/pages/loc-confirmation.test.tsx index 1e2f9d46e..6637c5596 100644 --- a/frontend/lib/tests/pages/loc-confirmation.test.tsx +++ b/frontend/lib/tests/pages/loc-confirmation.test.tsx @@ -10,7 +10,7 @@ describe('letter of complaint confirmation', () => { const createPal = (mailChoice: LetterRequestMailChoice) => new AppTesterPal(, { - url: Routes.loc.confirmation, + url: Routes.locale.loc.confirmation, session: { letterRequest: { updatedAt: "2018-09-14T01:42:12.829983+00:00", diff --git a/frontend/lib/tests/pages/onboarding-step-1.test.tsx b/frontend/lib/tests/pages/onboarding-step-1.test.tsx index cf493ba0c..3a52b6f58 100644 --- a/frontend/lib/tests/pages/onboarding-step-1.test.tsx +++ b/frontend/lib/tests/pages/onboarding-step-1.test.tsx @@ -8,7 +8,7 @@ import { FakeGeoResults } from '../util'; import Routes from '../../routes'; const PROPS = { - routes: Routes.onboarding, + routes: Routes.locale.onboarding, toCancel: '/cancel' }; diff --git a/frontend/lib/tests/pages/onboarding-step-2.test.tsx b/frontend/lib/tests/pages/onboarding-step-2.test.tsx index 9a6b80d14..42cc3b8ae 100644 --- a/frontend/lib/tests/pages/onboarding-step-2.test.tsx +++ b/frontend/lib/tests/pages/onboarding-step-2.test.tsx @@ -9,7 +9,7 @@ describe('onboarding step 2 page', () => { afterEach(AppTesterPal.cleanup); it('opens eviction modal', () => { - const pal = new AppTesterPal(); + const pal = new AppTesterPal(); const getDialog = () => pal.getDialogWithLabel(/You need legal help/i); // When we enable the checkbox, the dialog should show. diff --git a/frontend/lib/tests/pages/onboarding-step-3.test.tsx b/frontend/lib/tests/pages/onboarding-step-3.test.tsx index ad6d8d8b7..e297aaae0 100644 --- a/frontend/lib/tests/pages/onboarding-step-3.test.tsx +++ b/frontend/lib/tests/pages/onboarding-step-3.test.tsx @@ -9,7 +9,7 @@ import { getLeaseChoiceLabels } from '../../../../common-data/lease-choices'; const PROPS = { - routes: Routes.onboarding + routes: Routes.locale.onboarding }; const STEP_3 = new OnboardingStep3(PROPS); diff --git a/frontend/lib/tests/pages/onboarding-step-4.test.tsx b/frontend/lib/tests/pages/onboarding-step-4.test.tsx index bfa82b612..9b1070373 100644 --- a/frontend/lib/tests/pages/onboarding-step-4.test.tsx +++ b/frontend/lib/tests/pages/onboarding-step-4.test.tsx @@ -7,7 +7,7 @@ import Routes from '../../routes'; import { OnboardingInfoSignupIntent } from '../../queries/globalTypes'; const PROPS = { - routes: Routes.onboarding, + routes: Routes.locale.onboarding, toSuccess: '/success', signupIntent: OnboardingInfoSignupIntent.HP }; From 60ca507c801926e53f69a884d3321f44dae0d40f Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Wed, 6 Mar 2019 13:08:29 -0500 Subject: [PATCH 05/20] Add an i18n module and use it. --- frontend/lambda/lambda.tsx | 2 ++ frontend/lambda/tests/lambda.test.tsx | 3 +++ frontend/lib/hp-action.tsx | 10 +++---- frontend/lib/i18n.ts | 27 +++++++++++++++++++ frontend/lib/letter-of-complaint.tsx | 6 ++--- frontend/lib/main.ts | 2 ++ frontend/lib/pages/index-page.tsx | 6 ++--- frontend/lib/pages/landlord-details.tsx | 10 +++---- frontend/lib/progress-routes.tsx | 4 +-- frontend/lib/routes.ts | 25 ++++++++++++++--- frontend/lib/tests/hp-action.test.tsx | 4 +-- .../lib/tests/letter-of-complaint.test.tsx | 4 +-- frontend/lib/tests/progress-routes.test.tsx | 2 +- frontend/lib/tests/setup.ts | 3 +++ 14 files changed, 82 insertions(+), 26 deletions(-) create mode 100644 frontend/lib/i18n.ts diff --git a/frontend/lambda/lambda.tsx b/frontend/lambda/lambda.tsx index 60bae390d..efca4e947 100644 --- a/frontend/lambda/lambda.tsx +++ b/frontend/lambda/lambda.tsx @@ -24,6 +24,7 @@ import Helmet from 'react-helmet'; import { ErrorDisplay, getErrorString } from '../lib/error-boundary'; import { App, AppProps } from '../lib/app'; import { appStaticContextAsStaticRouterContext, AppStaticContext } from '../lib/app-static-context'; +import i18n from '../lib/i18n'; const readFile = promisify(fs.readFile); @@ -110,6 +111,7 @@ export function getBundleFiles(files: { file: string }[]): string[] { * lazy loading purposes. */ function generateResponse(event: AppProps, bundleStats: any): Promise { + i18n.initialize(''); return new Promise(resolve => { const context: AppStaticContext = { statusCode: 200, diff --git a/frontend/lambda/tests/lambda.test.tsx b/frontend/lambda/tests/lambda.test.tsx index 4432abb36..442a37737 100644 --- a/frontend/lambda/tests/lambda.test.tsx +++ b/frontend/lambda/tests/lambda.test.tsx @@ -5,6 +5,7 @@ import { Readable } from 'stream'; import { errorCatchingHandler, handleFromJSONStream, LambdaResponse, isPlainJsObject, getBundleFiles } from '../lambda'; import { AppProps } from '../../lib/app'; import { FakeServerInfo, FakeSessionInfo } from '../../lib/tests/util'; +import i18n from '../../lib/i18n'; const fakeAppProps: AppProps = { initialURL: '/', @@ -12,6 +13,8 @@ const fakeAppProps: AppProps = { initialSession: FakeSessionInfo }; +beforeEach(() => i18n.resetForTesting()); + test('lambda works', async () => { jest.setTimeout(10000); const response = await errorCatchingHandler(fakeAppProps); diff --git a/frontend/lib/hp-action.tsx b/frontend/lib/hp-action.tsx index cc1f8b8da..6e718889f 100644 --- a/frontend/lib/hp-action.tsx +++ b/frontend/lib/hp-action.tsx @@ -16,7 +16,7 @@ import { GetHPActionUploadStatus } from './queries/GetHPActionUploadStatus'; import { Redirect } from 'react-router'; import { SessionPoller } from './session-poller'; -const onboardingForHPActionRoute = Routes.locale.hp.onboarding.latestStep; +const onboardingForHPActionRoute = () => Routes.locale.hp.onboarding.latestStep; function HPActionSplash(): JSX.Element { return ( @@ -25,7 +25,7 @@ function HPActionSplash(): JSX.Element {

    Welcome to JustFix.nyc! This website will guide you through the process of starting an HP Action proceeding.

    An HP Action is a legal case you can bring against your landlord for failing to make repairs, not providing essential services, or harassing you.

    This service is free, secure, and confidential.

    - + Start my case @@ -174,7 +174,7 @@ const HPActionConfirmation = withAppContext((props: AppContextType) => { ); }); -export const HPActionProgressRoutesProps: ProgressRoutesProps = { +export const getHPActionProgressRoutesProps = (): ProgressRoutesProps => ({ toLatestStep: Routes.locale.hp.latestStep, label: "HP Action", welcomeSteps: [{ @@ -193,8 +193,8 @@ export const HPActionProgressRoutesProps: ProgressRoutesProps = { isComplete: (s) => s.hpActionUploadStatus === HPUploadStatus.SUCCEEDED }, { path: Routes.locale.hp.confirmation, exact: true, component: HPActionConfirmation} ] -}; +}); -const HPActionRoutes = buildProgressRoutesComponent(HPActionProgressRoutesProps); +const HPActionRoutes = buildProgressRoutesComponent(getHPActionProgressRoutesProps); export default HPActionRoutes; diff --git a/frontend/lib/i18n.ts b/frontend/lib/i18n.ts new file mode 100644 index 000000000..dc8ab4a99 --- /dev/null +++ b/frontend/lib/i18n.ts @@ -0,0 +1,27 @@ +class I18n { + private _locale: null|string = null; + + private raiseInitError(): never { + throw new Error('i18n is not initialized!'); + } + + get locale(): string { + if (this._locale === null) return this.raiseInitError(); + return this._locale; + } + + initialize(locale: string) { + if (this._locale !== null) { + throw new Error('i18n is already initialized!'); + } + this._locale = locale; + } + + resetForTesting(): void { + this._locale = null; + } +} + +const i18n = new I18n(); + +export default i18n; diff --git a/frontend/lib/letter-of-complaint.tsx b/frontend/lib/letter-of-complaint.tsx index 06a9148b1..2c5c27972 100644 --- a/frontend/lib/letter-of-complaint.tsx +++ b/frontend/lib/letter-of-complaint.tsx @@ -48,7 +48,7 @@ const LetterOfComplaintIssuesRoutes = () => ( /> ); -export const LOCProgressRoutesProps: ProgressRoutesProps = { +export const getLOCProgressRoutesProps = (): ProgressRoutesProps => ({ toLatestStep: Routes.locale.loc.latestStep, label: "Letter of Complaint", welcomeSteps: [{ @@ -64,8 +64,8 @@ export const LOCProgressRoutesProps: ProgressRoutesProps = { confirmationSteps: [{ path: Routes.locale.loc.confirmation, exact: true, component: LetterConfirmation }] -}; +}); -const LetterOfComplaintRoutes = buildProgressRoutesComponent(LOCProgressRoutesProps); +const LetterOfComplaintRoutes = buildProgressRoutesComponent(getLOCProgressRoutesProps); export default LetterOfComplaintRoutes; diff --git a/frontend/lib/main.ts b/frontend/lib/main.ts index 35db5268a..9dc6843e9 100644 --- a/frontend/lib/main.ts +++ b/frontend/lib/main.ts @@ -3,6 +3,7 @@ import { startApp, AppProps } from './app'; import { getElement } from './util'; import { ga } from './google-analytics'; +import i18n from './i18n'; function polyfillSmoothScroll() { @@ -46,6 +47,7 @@ window.addEventListener('load', () => { // Since JS is now loaded, let's remove that restriction. div.removeAttribute('hidden'); + i18n.initialize(''); startApp(div, initialProps); polyfillSmoothScroll(); showSafeModeUiOnShake(); diff --git a/frontend/lib/pages/index-page.tsx b/frontend/lib/pages/index-page.tsx index 2ae4f5ca2..c32cc07c3 100644 --- a/frontend/lib/pages/index-page.tsx +++ b/frontend/lib/pages/index-page.tsx @@ -13,7 +13,7 @@ export interface IndexPageProps { isLoggedIn: boolean; } -const onboardingForLOCRoute = Routes.locale.onboarding.latestStep; +const onboardingForLOCRoute = () => Routes.locale.onboarding.latestStep; export default class IndexPage extends React.Component { renderLoggedOut() { @@ -32,7 +32,7 @@ export default class IndexPage extends React.Component {

    JustFix.nyc is a free tool that notifies your landlord of repair issues via USPS Certified Mail®. Everything is documented, confidential, and secure.

    - + Start my free letter

    Already have an account? Sign in!

    @@ -69,7 +69,7 @@ export default class IndexPage extends React.Component {
    - + Start my free letter diff --git a/frontend/lib/pages/landlord-details.tsx b/frontend/lib/pages/landlord-details.tsx index 7b987c885..3a3724ada 100644 --- a/frontend/lib/pages/landlord-details.tsx +++ b/frontend/lib/pages/landlord-details.tsx @@ -19,9 +19,9 @@ const BLANK_INPUT: LandlordDetailsInput = { address: '' }; -const PREV_STEP = Routes.locale.loc.accessDates; +const PREV_STEP = () => Routes.locale.loc.accessDates; -const NEXT_STEP = Routes.locale.loc.preview; +const NEXT_STEP = () => Routes.locale.loc.preview; function renderForm(ctx: FormContext): JSX.Element { return ( @@ -29,7 +29,7 @@ function renderForm(ctx: FormContext): JSX.Element {
    - +
    @@ -68,8 +68,8 @@ function ReadOnlyLandlordDetails(props: {details: AllSessionInfo_landlordDetails
    {splitLines(details.address)}
    - - Preview letter + + Preview letter
    ); diff --git a/frontend/lib/progress-routes.tsx b/frontend/lib/progress-routes.tsx index 3dfd21d61..73ac08b33 100644 --- a/frontend/lib/progress-routes.tsx +++ b/frontend/lib/progress-routes.tsx @@ -78,6 +78,6 @@ export function ProgressRoutes(props: ProgressRoutesProps): JSX.Element { ); } -export function buildProgressRoutesComponent(props: ProgressRoutesProps): () => JSX.Element { - return () => ; +export function buildProgressRoutesComponent(getProps: () => ProgressRoutesProps): () => JSX.Element { + return () => ; } diff --git a/frontend/lib/routes.ts b/frontend/lib/routes.ts index 34c58b105..0525d2739 100644 --- a/frontend/lib/routes.ts +++ b/frontend/lib/routes.ts @@ -1,5 +1,6 @@ import { matchPath, RouteComponentProps } from 'react-router-dom'; import { OnboardingInfoSignupIntent } from './queries/globalTypes'; +import i18n from './i18n'; /** * Metadata about signup intents. @@ -145,12 +146,20 @@ function createLocalizedRouteInfo(prefix: string) { } } +let currentLocaleRoutes: LocalizedRouteInfo|null = null; + /** * This is an ad-hoc structure that defines URL routes for our app. */ const Routes = { /** Localized routes for the user's currently-selected locale. */ - locale: createLocalizedRouteInfo(''), + get locale(): LocalizedRouteInfo { + if (currentLocaleRoutes === null) { + const localePrefix = i18n.locale === '' ? '' : `/${i18n.locale}`; + currentLocaleRoutes = createLocalizedRouteInfo(localePrefix); + } + return currentLocaleRoutes; + }, /** * The *admin* login page. We override Django's default admin login @@ -207,9 +216,16 @@ export function isParameterizedRoute(path: string): boolean { export class RouteMap { private existenceMap: Map = new Map(); private parameterizedRoutes: string[] = []; + private isInitialized = false; - constructor(routes: any) { - this.populate(routes); + constructor(private readonly routes: any) { + } + + private ensureIsInitialized() { + if (!this.isInitialized) { + this.populate(this.routes); + this.isInitialized = true; + } } private populate(routes: any) { @@ -228,6 +244,7 @@ export class RouteMap { } get size(): number { + this.ensureIsInitialized(); return this.existenceMap.size + this.parameterizedRoutes.length; } @@ -235,6 +252,7 @@ export class RouteMap { * Return an iterator that yields all routes that don't have parameters. */ nonParameterizedRoutes(): IterableIterator { + this.ensureIsInitialized(); return this.existenceMap.keys(); } @@ -249,6 +267,7 @@ export class RouteMap { * further down the view heirarchy to resolve. */ exists(pathname: string): boolean { + this.ensureIsInitialized(); if (this.existenceMap.has(pathname)) { return true; } diff --git a/frontend/lib/tests/hp-action.test.tsx b/frontend/lib/tests/hp-action.test.tsx index c4ab3bfc2..2679c0e69 100644 --- a/frontend/lib/tests/hp-action.test.tsx +++ b/frontend/lib/tests/hp-action.test.tsx @@ -1,12 +1,12 @@ import React from 'react'; import { AppTesterPal } from "./app-tester-pal"; -import HPActionRoutes, { HPActionProgressRoutesProps } from '../hp-action'; +import HPActionRoutes, { getHPActionProgressRoutesProps } from '../hp-action'; import { ProgressRoutesTester } from './progress-routes-tester'; import Routes from '../routes'; import { HPUploadStatus } from '../queries/globalTypes'; -const tester = new ProgressRoutesTester(HPActionProgressRoutesProps, 'HP Action'); +const tester = new ProgressRoutesTester(getHPActionProgressRoutesProps(), 'HP Action'); tester.defineSmokeTests(); diff --git a/frontend/lib/tests/letter-of-complaint.test.tsx b/frontend/lib/tests/letter-of-complaint.test.tsx index 86b0f556a..e3d777d65 100644 --- a/frontend/lib/tests/letter-of-complaint.test.tsx +++ b/frontend/lib/tests/letter-of-complaint.test.tsx @@ -1,8 +1,8 @@ -import { LOCProgressRoutesProps } from '../letter-of-complaint'; +import { getLOCProgressRoutesProps } from '../letter-of-complaint'; import Routes from '../routes'; import { ProgressRoutesTester } from './progress-routes-tester'; -const tester = new ProgressRoutesTester(LOCProgressRoutesProps, 'letter of complaint'); +const tester = new ProgressRoutesTester(getLOCProgressRoutesProps(), 'letter of complaint'); tester.defineSmokeTests(); diff --git a/frontend/lib/tests/progress-routes.test.tsx b/frontend/lib/tests/progress-routes.test.tsx index d448617cd..299fb7877 100644 --- a/frontend/lib/tests/progress-routes.test.tsx +++ b/frontend/lib/tests/progress-routes.test.tsx @@ -13,7 +13,7 @@ const myRoutesProps: ProgressRoutesProps = { confirmationSteps: [] }; -const MyRoutes = buildProgressRoutesComponent(myRoutesProps); +const MyRoutes = buildProgressRoutesComponent(() => myRoutesProps); describe("ProgressRoutes", () => { afterEach(AppTesterPal.cleanup); diff --git a/frontend/lib/tests/setup.ts b/frontend/lib/tests/setup.ts index 782cae417..98c27b5ea 100644 --- a/frontend/lib/tests/setup.ts +++ b/frontend/lib/tests/setup.ts @@ -5,9 +5,12 @@ import { defaultContext } from '../app-context'; import { FakeAppContext } from './util'; import chalk from 'chalk'; import './confetti.setup'; +import i18n from '../i18n'; configure({ adapter: new Adapter() }); +i18n.initialize(''); + Object.keys(FakeAppContext).forEach(prop => { Object.defineProperty(defaultContext, prop, { value: (FakeAppContext as any)[prop] From b292857aa232b863152a632b1e607ba134ffbca2 Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Wed, 6 Mar 2019 13:25:26 -0500 Subject: [PATCH 06/20] Pass locale from server to client. --- frontend/lambda/lambda.tsx | 2 +- frontend/lambda/tests/lambda.test.tsx | 1 + frontend/lib/app.tsx | 3 +++ frontend/lib/main.ts | 2 +- frontend/lib/tests/app.test.tsx | 1 + project/views.py | 1 + 6 files changed, 8 insertions(+), 2 deletions(-) diff --git a/frontend/lambda/lambda.tsx b/frontend/lambda/lambda.tsx index efca4e947..cd048f0d5 100644 --- a/frontend/lambda/lambda.tsx +++ b/frontend/lambda/lambda.tsx @@ -111,7 +111,7 @@ export function getBundleFiles(files: { file: string }[]): string[] { * lazy loading purposes. */ function generateResponse(event: AppProps, bundleStats: any): Promise { - i18n.initialize(''); + i18n.initialize(event.locale); return new Promise(resolve => { const context: AppStaticContext = { statusCode: 200, diff --git a/frontend/lambda/tests/lambda.test.tsx b/frontend/lambda/tests/lambda.test.tsx index 442a37737..160e10ef1 100644 --- a/frontend/lambda/tests/lambda.test.tsx +++ b/frontend/lambda/tests/lambda.test.tsx @@ -9,6 +9,7 @@ import i18n from '../../lib/i18n'; const fakeAppProps: AppProps = { initialURL: '/', + locale: '', server: FakeServerInfo, initialSession: FakeSessionInfo }; diff --git a/frontend/lib/app.tsx b/frontend/lib/app.tsx index 7e04a9d8d..9c3eb0128 100644 --- a/frontend/lib/app.tsx +++ b/frontend/lib/app.tsx @@ -28,6 +28,9 @@ export interface AppProps { /** The initial URL to render on page load. */ initialURL: string; + /** The locale the user is on. */ + locale: string; + /** The initial session state the App was started with. */ initialSession: AllSessionInfo; diff --git a/frontend/lib/main.ts b/frontend/lib/main.ts index 9dc6843e9..fbdda7ca3 100644 --- a/frontend/lib/main.ts +++ b/frontend/lib/main.ts @@ -47,7 +47,7 @@ window.addEventListener('load', () => { // Since JS is now loaded, let's remove that restriction. div.removeAttribute('hidden'); - i18n.initialize(''); + i18n.initialize(initialProps.locale); startApp(div, initialProps); polyfillSmoothScroll(); showSafeModeUiOnShake(); diff --git a/frontend/lib/tests/app.test.tsx b/frontend/lib/tests/app.test.tsx index ceab64c23..381efea99 100644 --- a/frontend/lib/tests/app.test.tsx +++ b/frontend/lib/tests/app.test.tsx @@ -9,6 +9,7 @@ describe('AppWithoutRouter', () => { const { client } = createTestGraphQlClient(); const props: AppPropsWithRouter = { initialURL: '/', + locale: '', initialSession: FakeSessionInfo, server: FakeServerInfo, history: {} as any, diff --git a/project/views.py b/project/views.py index 1e21e4e0b..cfe3ee78d 100644 --- a/project/views.py +++ b/project/views.py @@ -150,6 +150,7 @@ def react_rendered_view(request, url: str): initial_props = { 'initialURL': url, 'initialSession': get_initial_session(request), + 'locale': '', 'server': { 'originURL': request.build_absolute_uri('/')[:-1], 'staticURL': settings.STATIC_URL, From ac65b1c4ddeee3aad0d1e38de63b5edc52f8a9c4 Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Wed, 6 Mar 2019 17:12:55 -0500 Subject: [PATCH 07/20] Redirect non-localized paths to localized ones. --- frontend/lib/app.tsx | 8 +++++--- frontend/lib/i18n.ts | 11 ++++++++++- frontend/lib/routes.ts | 24 +++++++++++++++++++----- 3 files changed, 34 insertions(+), 9 deletions(-) diff --git a/frontend/lib/app.tsx b/frontend/lib/app.tsx index 9c3eb0128..c731ca5b4 100644 --- a/frontend/lib/app.tsx +++ b/frontend/lib/app.tsx @@ -1,7 +1,7 @@ import React, { RefObject } from 'react'; import ReactDOM from 'react-dom'; import autobind from 'autobind-decorator'; -import { BrowserRouter, Switch, Route, RouteComponentProps, withRouter } from 'react-router-dom'; +import { BrowserRouter, Switch, Route, RouteComponentProps, withRouter, Redirect } from 'react-router-dom'; import Loadable from 'react-loadable'; import GraphQlClient from './graphql-client'; @@ -224,10 +224,12 @@ export class AppWithoutRouter extends React.Component { - if (routeMap.exists(props.location.pathname)) { + const { pathname } = props.location; + if (routeMap.exists(pathname)) { return this.renderRoutes(props.location); } - return NotFound(props); + const redirect = routeMap.getNearestRedirect(pathname); + return redirect ? : NotFound(props); }}/> diff --git a/frontend/lib/i18n.ts b/frontend/lib/i18n.ts index dc8ab4a99..c8fa76cf6 100644 --- a/frontend/lib/i18n.ts +++ b/frontend/lib/i18n.ts @@ -1,4 +1,4 @@ -class I18n { +export class I18n { private _locale: null|string = null; private raiseInitError(): never { @@ -10,6 +10,11 @@ class I18n { return this._locale; } + get localePathPrefix(): string { + const { locale } = this; + return locale === '' ? '' : `/${locale}`; + } + initialize(locale: string) { if (this._locale !== null) { throw new Error('i18n is already initialized!'); @@ -17,6 +22,10 @@ class I18n { this._locale = locale; } + get isInitialized(): boolean { + return this._locale !== null; + } + resetForTesting(): void { this._locale = null; } diff --git a/frontend/lib/routes.ts b/frontend/lib/routes.ts index 0525d2739..06da1217a 100644 --- a/frontend/lib/routes.ts +++ b/frontend/lib/routes.ts @@ -1,6 +1,6 @@ import { matchPath, RouteComponentProps } from 'react-router-dom'; import { OnboardingInfoSignupIntent } from './queries/globalTypes'; -import i18n from './i18n'; +import i18n, { I18n } from './i18n'; /** * Metadata about signup intents. @@ -155,8 +155,7 @@ const Routes = { /** Localized routes for the user's currently-selected locale. */ get locale(): LocalizedRouteInfo { if (currentLocaleRoutes === null) { - const localePrefix = i18n.locale === '' ? '' : `/${i18n.locale}`; - currentLocaleRoutes = createLocalizedRouteInfo(localePrefix); + currentLocaleRoutes = createLocalizedRouteInfo(i18n.localePathPrefix); } return currentLocaleRoutes; }, @@ -218,7 +217,7 @@ export class RouteMap { private parameterizedRoutes: string[] = []; private isInitialized = false; - constructor(private readonly routes: any) { + constructor(private readonly routes: any, private readonly i18n: I18n = new I18n()) { } private ensureIsInitialized() { @@ -279,6 +278,21 @@ export class RouteMap { } return false; } + + /** + * If the given concrete pathname doesn't exist, see if there's + * a nearby pathname that *does* match, which we can reasonably + * redirect the user to. + */ + getNearestRedirect(pathname: string): string|null { + if (this.i18n.isInitialized && this.i18n.localePathPrefix) { + const prefixWithPath = this.i18n.localePathPrefix + pathname; + if (this.exists(prefixWithPath)) { + return prefixWithPath; + } + } + return null; + } } -export const routeMap = new RouteMap(Routes); +export const routeMap = new RouteMap(Routes, i18n); From 02b4b887c7d7c36051f0a4e6b253713cd6085e1e Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Wed, 6 Mar 2019 17:19:51 -0500 Subject: [PATCH 08/20] Factor out App.renderRoute(). --- frontend/lib/app.tsx | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/frontend/lib/app.tsx b/frontend/lib/app.tsx index c731ca5b4..0713e67c5 100644 --- a/frontend/lib/app.tsx +++ b/frontend/lib/app.tsx @@ -208,6 +208,15 @@ export class AppWithoutRouter extends React.Component): JSX.Element { + const { pathname } = props.location; + if (routeMap.exists(pathname)) { + return this.renderRoutes(props.location); + } + const redirect = routeMap.getNearestRedirect(pathname); + return redirect ? : NotFound(props); + } + render() { if (this.props.modal) { return @@ -223,14 +232,7 @@ export class AppWithoutRouter extends React.Component - { - const { pathname } = props.location; - if (routeMap.exists(pathname)) { - return this.renderRoutes(props.location); - } - const redirect = routeMap.getNearestRedirect(pathname); - return redirect ? : NotFound(props); - }}/> + this.renderRoute(props)}/> From ead99a201cfde62b2b76d84d966a30ead575906e Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Wed, 6 Mar 2019 17:33:03 -0500 Subject: [PATCH 09/20] Add a bunch of docs to i18n.ts. --- frontend/lib/i18n.ts | 45 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/frontend/lib/i18n.ts b/frontend/lib/i18n.ts index c8fa76cf6..8c371e373 100644 --- a/frontend/lib/i18n.ts +++ b/frontend/lib/i18n.ts @@ -1,3 +1,10 @@ +/** + * This class keeps track of internationalization-related data. + * + * Instances start out uninitialized, and must be explicitly + * initialized before any other methods or properties can be + * accessed. + */ export class I18n { private _locale: null|string = null; @@ -5,16 +12,35 @@ export class I18n { throw new Error('i18n is not initialized!'); } + /** + * Return the current locale, raising an error if the + * class is uninitialized. + * + * If the locale is set to the empty string, it means that + * localization is currently disabled. Otherwise, the + * string will be an ISO 639-1 code such as 'en' or 'es'. + */ get locale(): string { if (this._locale === null) return this.raiseInitError(); return this._locale; } + /** + * Return the URL path prefix for the current locale. + * If localization is disabled, this will be the + * empty string; otherwise, it will be a slash followed + * by the locale's ISO 639-1 code, e.g. '/en'. + */ get localePathPrefix(): string { const { locale } = this; return locale === '' ? '' : `/${locale}`; } + /** + * Initialize the instance to the given locale. Pass + * an empty string to indicate that localization is + * disabled, or an ISO 639-1 code such as 'en' or 'es'. + */ initialize(locale: string) { if (this._locale !== null) { throw new Error('i18n is already initialized!'); @@ -22,15 +48,34 @@ export class I18n { this._locale = locale; } + /** Return whether the instance is initialized. */ get isInitialized(): boolean { return this._locale !== null; } + /** + * Reset the instance, reverting it to an uninitialized + * state. + * + * As the name implies, this should ONLY be used for testing. + */ resetForTesting(): void { this._locale = null; } } +/** + * This is a global singleton for the JS runtime. It's largely + * a singleton because passing it around everywhere would be + * a massive headache, especially given the state of the codebase + * at the time that internationalization was introduced. + * + * That said, one should prefer to write client code in a way + * such that an I18n object is passed into it, rather than + * grabbing this singleton directly. This will make it easier + * to unit test, as well as to eventually get rid of the global + * singleton. + */ const i18n = new I18n(); export default i18n; From 29e867612b71c5a894286cd6a530c1bd5067b977 Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Wed, 6 Mar 2019 18:02:09 -0500 Subject: [PATCH 10/20] Add tests. --- frontend/lib/i18n.ts | 19 ++++++++++++++++--- frontend/lib/tests/i18n.test.ts | 29 +++++++++++++++++++++++++++++ frontend/lib/tests/routes.test.ts | 13 +++++++++++++ 3 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 frontend/lib/tests/i18n.test.ts diff --git a/frontend/lib/i18n.ts b/frontend/lib/i18n.ts index 8c371e373..6d729f22b 100644 --- a/frontend/lib/i18n.ts +++ b/frontend/lib/i18n.ts @@ -8,6 +8,18 @@ export class I18n { private _locale: null|string = null; + /** + * Create an instance, optionally auto-initializing it. + * + * @param locale An empty string to indicate that localization is + * disabled, or an ISO 639-1 code such as 'en' or 'es'. + */ + constructor(locale?: string) { + if (typeof(locale) === 'string') { + this.initialize(locale); + } + } + private raiseInitError(): never { throw new Error('i18n is not initialized!'); } @@ -37,9 +49,10 @@ export class I18n { } /** - * Initialize the instance to the given locale. Pass - * an empty string to indicate that localization is - * disabled, or an ISO 639-1 code such as 'en' or 'es'. + * Initialize the instance to the given locale. + * + * @param locale An empty string to indicate that localization is + * disabled, or an ISO 639-1 code such as 'en' or 'es'. */ initialize(locale: string) { if (this._locale !== null) { diff --git a/frontend/lib/tests/i18n.test.ts b/frontend/lib/tests/i18n.test.ts new file mode 100644 index 000000000..110ec2dc9 --- /dev/null +++ b/frontend/lib/tests/i18n.test.ts @@ -0,0 +1,29 @@ +import i18n, { I18n } from "../i18n"; + +const UNINIT_RE = /i18n is not initialized/i; + +describe('I18n', () => { + it('raises exception on methods/properties if uninitialized', () => { + const i18n = new I18n(); + expect(() => i18n.locale).toThrow(UNINIT_RE); + expect(() => i18n.localePathPrefix).toThrow(UNINIT_RE); + }); + + it('returns whether it is initialized or not', () => { + expect(new I18n('en').isInitialized).toBe(true); + + const i18n = new I18n(); + expect(i18n.isInitialized).toBe(false); + i18n.initialize('en'); + expect(i18n.isInitialized).toBe(true); + }); + + it('raises an error if initialized twice', () => { + expect(() => new I18n('en').initialize('es')).toThrow(/already initialized/i); + }); + + it('returns locale path prefixes', () => { + expect(new I18n('').localePathPrefix).toBe(''); + expect(new I18n('en').localePathPrefix).toBe('/en'); + }); +}); diff --git a/frontend/lib/tests/routes.test.ts b/frontend/lib/tests/routes.test.ts index 5c577077e..880e91daf 100644 --- a/frontend/lib/tests/routes.test.ts +++ b/frontend/lib/tests/routes.test.ts @@ -1,5 +1,6 @@ import { isModalRoute, RouteMap, getSignupIntentOnboardingInfo } from "../routes"; import { OnboardingInfoSignupIntent } from "../queries/globalTypes"; +import { I18n } from "../i18n"; test('isModalRoute() works', () => { expect(isModalRoute('/blah')).toBe(false); @@ -47,4 +48,16 @@ describe('RouteMap', () => { expect(map.exists('/blah/7')).toBe(true); expect(map.exists('/blah/9/zorp')).toBe(false); }); + + describe('getNearestRedirect()', () => { + it('returns null if no redirect is found', () => { + const map = new RouteMap({ blah: '/blah' }); + expect(map.getNearestRedirect('/zzz')).toBeNull(); + }); + + it('returns localized redirects', () => { + const map = new RouteMap({ blah: '/en/blah' }, new I18n('en')); + expect(map.getNearestRedirect('/blah')).toBe('/en/blah'); + }); + }); }); From aae44a591181145bee286b9bd8ad6b7c17b8c0ca Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Wed, 6 Mar 2019 18:04:55 -0500 Subject: [PATCH 11/20] Fix linting error. --- frontend/lib/tests/i18n.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/lib/tests/i18n.test.ts b/frontend/lib/tests/i18n.test.ts index 110ec2dc9..428f068b7 100644 --- a/frontend/lib/tests/i18n.test.ts +++ b/frontend/lib/tests/i18n.test.ts @@ -1,4 +1,4 @@ -import i18n, { I18n } from "../i18n"; +import { I18n } from "../i18n"; const UNINIT_RE = /i18n is not initialized/i; From e8492dd285ea40a32bd2560cfddd89dc5a262468 Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Wed, 6 Mar 2019 18:09:52 -0500 Subject: [PATCH 12/20] Add more comments. --- frontend/lib/app.tsx | 6 +++++- project/views.py | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/frontend/lib/app.tsx b/frontend/lib/app.tsx index 0713e67c5..2702cd023 100644 --- a/frontend/lib/app.tsx +++ b/frontend/lib/app.tsx @@ -28,7 +28,11 @@ export interface AppProps { /** The initial URL to render on page load. */ initialURL: string; - /** The locale the user is on. */ + /** + * The locale the user is on. This can be an empty string to + * indicate that localization is disabled, or an ISO 639-1 + * code such as 'en' or 'es'. + */ locale: string; /** The initial session state the App was started with. */ diff --git a/project/views.py b/project/views.py index cfe3ee78d..e079692bc 100644 --- a/project/views.py +++ b/project/views.py @@ -150,7 +150,7 @@ def react_rendered_view(request, url: str): initial_props = { 'initialURL': url, 'initialSession': get_initial_session(request), - 'locale': '', + 'locale': '', # Disable localization for now. 'server': { 'originURL': request.build_absolute_uri('/')[:-1], 'staticURL': settings.STATIC_URL, From ea77e3b205fb2347430cf03afbc941841403595e Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Thu, 7 Mar 2019 07:27:49 -0500 Subject: [PATCH 13/20] Enable localization on the Django side. --- frontend/lib/dev.tsx | 2 +- frontend/lib/routes.ts | 2 +- hpaction/tests/test_views.py | 22 ++++++++++++++++++---- loc/tests/test_views.py | 11 +++++++++-- project/settings.py | 5 +++++ project/tests/test_views.py | 26 ++++++++++++++++---------- project/urls.py | 14 +++++++++++--- project/views.py | 8 +++++--- 8 files changed, 66 insertions(+), 24 deletions(-) diff --git a/frontend/lib/dev.tsx b/frontend/lib/dev.tsx index 57beda3f5..2b41a736e 100644 --- a/frontend/lib/dev.tsx +++ b/frontend/lib/dev.tsx @@ -75,7 +75,7 @@ export default function DevRoutes(): JSX.Element { return ( - } /> + } /> diff --git a/frontend/lib/routes.ts b/frontend/lib/routes.ts index 06da1217a..e475a4d56 100644 --- a/frontend/lib/routes.ts +++ b/frontend/lib/routes.ts @@ -173,7 +173,7 @@ const Routes = { */ dev: { [ROUTE_PREFIX]: '/dev', - home: '/dev', + home: '/dev/', examples: { [ROUTE_PREFIX]: '/dev/examples', redirect: '/dev/examples/redirect', diff --git a/hpaction/tests/test_views.py b/hpaction/tests/test_views.py index aa3901b91..b6f3fac67 100644 --- a/hpaction/tests/test_views.py +++ b/hpaction/tests/test_views.py @@ -24,10 +24,17 @@ def test_decode_lhi_b64_data_works_with_alt_chars(): class TestUpload: - def test_it_returns_404_when_token_is_invalid(self, db, client): + def test_it_fails_when_token_is_invalid(self, db, client): url = reverse('hpaction:upload', kwargs={'token_str': 'blarg'}) res = client.post(url) - assert res.status_code == 404 + + # Unfortunately, right now 404's returned by non-i18n routes appear to + # be converted to redirects to locale-prefixed routes. This is annoying + # but it's ultimately okay since it will result in a 404 on the + # locale-prefixed route. We won't test that part because it will make + # the test take longer, though. + assert res.status_code == 302 + assert res.url == "/en/hp/upload/blarg" def test_it_works(self, db, client, django_file_storage): token = UploadTokenFactory() @@ -49,9 +56,16 @@ class TestLatestPDF: def setup(self): self.url = reverse('hpaction:latest_pdf') - def test_it_returns_404_when_no_pdfs_exist(self, admin_client): + def test_it_fails_when_no_pdfs_exist(self, admin_client): res = admin_client.get(self.url) - assert res.status_code == 404 + + # Unfortunately, right now 404's returned by non-i18n routes appear to + # be converted to redirects to locale-prefixed routes. This is annoying + # but it's ultimately okay since it will result in a 404 on the + # locale-prefixed route. We won't test that part because it will make + # the test take longer, though. + assert res.status_code == 302 + assert res.url == "/en/hp/latest.pdf" def test_it_returns_pdf(self, client, db, django_file_storage): docs = HPActionDocumentsFactory() diff --git a/loc/tests/test_views.py b/loc/tests/test_views.py index b78cd8e48..19b828d6b 100644 --- a/loc/tests/test_views.py +++ b/loc/tests/test_views.py @@ -127,9 +127,16 @@ def test_admin_letter_pdf_works(outreach_client): assert res['Content-Type'] == 'application/pdf' -def test_admin_letter_pdf_returns_404_for_nonexistent_users(admin_client): +def test_admin_letter_pdf_fails_for_nonexistent_users(admin_client): res = admin_client.get(f'/loc/admin/1024/letter.pdf') - assert res.status_code == 404 + + # Unfortunately, right now 404's returned by non-i18n routes appear to + # be converted to redirects to locale-prefixed routes. This is annoying + # but it's ultimately okay since it will result in a 404 on the + # locale-prefixed route. We won't test that part because it will make + # the test take longer, though. + assert res.status_code == 302 + assert res.url == "/en/loc/admin/1024/letter.pdf" @pytest.mark.django_db diff --git a/project/settings.py b/project/settings.py index fa9cce5f0..bfa4512c6 100644 --- a/project/settings.py +++ b/project/settings.py @@ -72,6 +72,7 @@ 'django.middleware.security.SecurityMiddleware', 'whitenoise.middleware.WhiteNoiseMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', + 'django.middleware.locale.LocaleMiddleware', 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', @@ -179,6 +180,10 @@ LANGUAGE_CODE = 'en-us' +LANGUAGES = [ + ('en', 'English'), +] + TIME_ZONE = 'UTC' USE_I18N = True diff --git a/project/tests/test_views.py b/project/tests/test_views.py index e9bbee2c1..b9a02eecd 100644 --- a/project/tests/test_views.py +++ b/project/tests/test_views.py @@ -45,7 +45,7 @@ def test_get_initial_session_works(graphql_client): def test_invalid_post_returns_400(client): - response = client.post('/') + response = client.post('/en/') assert response.status_code == 400 assert response.content == b'No GraphQL query found' @@ -56,7 +56,7 @@ def test_invalid_post_returns_400(client): def test_index_works_when_not_in_safe_mode(client): - response = client.get('/') + response = client.get('/en/') assert response.status_code == 200 assert 'JustFix.nyc' in response.context['title_tag'] assert '', example_server_error), + re_path(r'^.*$', react_rendered_view), +], 'dev') + urlpatterns = [ path('verify', twofactor.views.verify, name='verify'), path('health', health), - path('admin/login/', react_rendered_view, kwargs={'url': 'admin/login/'}), + path('admin/login/', react_rendered_view), path('admin/', admin.site.urls), path('loc/', include('loc.urls')), path('hp/', include('hpaction.urls')), path('safe-mode/', include('frontend.safe_mode')), path('legacy-app', redirect_to_legacy_app, name='redirect-to-legacy-app'), path('favicon.ico', redirect_favicon), - path('dev/examples/server-error/', example_server_error), path('graphql', GraphQLView.as_view(batch=True), name='batch-graphql'), + path('dev/', include(dev_patterns, namespace='dev')), ] if settings.DEBUG: @@ -47,4 +53,6 @@ urlpatterns.append( path('graphiql', GraphQLView.as_view(graphiql=True))) -urlpatterns.append(re_path(r'^(?P.*)$', react_rendered_view)) +urlpatterns += i18n_patterns( + re_path(r'.*$', react_rendered_view), +) diff --git a/project/views.py b/project/views.py index e079692bc..fb3aad090 100644 --- a/project/views.py +++ b/project/views.py @@ -5,6 +5,7 @@ from django.views.decorators.http import require_POST from django.views.decorators.csrf import csrf_exempt from django.utils.safestring import SafeString +from django.utils import translation from django.shortcuts import render, redirect from django.urls import reverse from django.conf import settings @@ -137,8 +138,9 @@ def get_legacy_form_submission(request): } -def react_rendered_view(request, url: str): - url = f'/{url}' +def react_rendered_view(request): + url = request.path + cur_language = translation.get_language_from_request(request, check_path=True) querystring = request.GET.urlencode() if querystring: url += f'?{querystring}' @@ -150,7 +152,7 @@ def react_rendered_view(request, url: str): initial_props = { 'initialURL': url, 'initialSession': get_initial_session(request), - 'locale': '', # Disable localization for now. + 'locale': cur_language, 'server': { 'originURL': request.build_absolute_uri('/')[:-1], 'staticURL': settings.STATIC_URL, From 81d48d00e81643310657bbd2e2a16df7d5662c46 Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Thu, 7 Mar 2019 09:58:13 -0500 Subject: [PATCH 14/20] Add ENABLE_I18N env var, make tests independent of it. --- airtable/tests/test_record.py | 5 ++-- hpaction/tests/test_models.py | 4 ++- hpaction/tests/test_views.py | 22 +++------------ loc/tests/test_admin.py | 2 +- loc/tests/test_views.py | 49 +++++++++++++++++++--------------- loc/urls.py | 2 +- project/justfix_environment.py | 3 +++ project/settings.py | 6 ++--- project/tests/test_views.py | 31 +++++++++++++-------- project/urls.py | 6 ++--- project/views.py | 4 ++- 11 files changed, 72 insertions(+), 62 deletions(-) diff --git a/airtable/tests/test_record.py b/airtable/tests/test_record.py index 78ca94e5e..122e5b41e 100644 --- a/airtable/tests/test_record.py +++ b/airtable/tests/test_record.py @@ -48,8 +48,9 @@ def test_from_user_works_with_letter_request(): fields = Fields.from_user(lr.user) assert fields.letter_request__will_we_mail is True assert fields.letter_request__created_at == datetime.datetime.utcnow().date().isoformat() - assert fields.letter_request__admin_pdf_url == \ - f'https://example.com/loc/admin/{lr.user.pk}/letter.pdf' + url = fields.letter_request__admin_pdf_url + assert url.startswith('https://example.com/') + assert url.endswith(f'/loc/admin/{lr.user.pk}/letter.pdf') @pytest.mark.django_db diff --git a/hpaction/tests/test_models.py b/hpaction/tests/test_models.py index e387b92ba..665ee7170 100644 --- a/hpaction/tests/test_models.py +++ b/hpaction/tests/test_models.py @@ -41,7 +41,9 @@ def test_create_documents_from_works(self, db, django_file_storage): def test_get_upload_url_works(self, db): token = UploadToken(id='boop') - assert token.get_upload_url() == 'https://example.com/hp/upload/boop' + url = token.get_upload_url() + assert url.startswith('https://example.com/') + assert url.endswith('/hp/upload/boop') class TestHPActionDocuments: diff --git a/hpaction/tests/test_views.py b/hpaction/tests/test_views.py index b6f3fac67..aa3901b91 100644 --- a/hpaction/tests/test_views.py +++ b/hpaction/tests/test_views.py @@ -24,17 +24,10 @@ def test_decode_lhi_b64_data_works_with_alt_chars(): class TestUpload: - def test_it_fails_when_token_is_invalid(self, db, client): + def test_it_returns_404_when_token_is_invalid(self, db, client): url = reverse('hpaction:upload', kwargs={'token_str': 'blarg'}) res = client.post(url) - - # Unfortunately, right now 404's returned by non-i18n routes appear to - # be converted to redirects to locale-prefixed routes. This is annoying - # but it's ultimately okay since it will result in a 404 on the - # locale-prefixed route. We won't test that part because it will make - # the test take longer, though. - assert res.status_code == 302 - assert res.url == "/en/hp/upload/blarg" + assert res.status_code == 404 def test_it_works(self, db, client, django_file_storage): token = UploadTokenFactory() @@ -56,16 +49,9 @@ class TestLatestPDF: def setup(self): self.url = reverse('hpaction:latest_pdf') - def test_it_fails_when_no_pdfs_exist(self, admin_client): + def test_it_returns_404_when_no_pdfs_exist(self, admin_client): res = admin_client.get(self.url) - - # Unfortunately, right now 404's returned by non-i18n routes appear to - # be converted to redirects to locale-prefixed routes. This is annoying - # but it's ultimately okay since it will result in a 404 on the - # locale-prefixed route. We won't test that part because it will make - # the test take longer, though. - assert res.status_code == 302 - assert res.url == "/en/hp/latest.pdf" + assert res.status_code == 404 def test_it_returns_pdf(self, client, db, django_file_storage): docs = HPActionDocumentsFactory() diff --git a/loc/tests/test_admin.py b/loc/tests/test_admin.py index 1e9e9d4d9..dbd04528b 100644 --- a/loc/tests/test_admin.py +++ b/loc/tests/test_admin.py @@ -25,4 +25,4 @@ def test_loc_actions_shows_pdf_link_when_user_has_letter_request(): def test_print_loc_envelopes_works(): user = UserFactory() redirect = print_loc_envelopes(None, None, JustfixUser.objects.all()) - assert redirect.url == f'/loc/admin/envelopes.pdf?user_ids={user.pk}' + assert redirect.url.endswith(f'/loc/admin/envelopes.pdf?user_ids={user.pk}') diff --git a/loc/tests/test_views.py b/loc/tests/test_views.py index 19b828d6b..89333d3f7 100644 --- a/loc/tests/test_views.py +++ b/loc/tests/test_views.py @@ -1,4 +1,5 @@ from functools import wraps +from django.urls import reverse import pytest from users.tests.factories import UserFactory @@ -22,6 +23,18 @@ "ignore:The html argument of XMLParser") +def example_url(format: str) -> str: + return reverse('loc_example', args=(format,)) + + +def letter_url(format: str) -> str: + return reverse('loc', args=(format,)) + + +def admin_letter_url(user_id: int) -> str: + return reverse('loc_for_user', kwargs={'user_id': user_id}) + + def requires_pdf_rendering(fn): @pytest.mark.skipif(not can_we_render_pdfs(), reason='PDF generation is unsupported') @@ -59,12 +72,12 @@ def test_render_document_raises_err_on_invalid_format(): def test_letter_requires_login(client): - res = client.get('/loc/letter.html') + res = client.get(letter_url('html')) assert res.status_code == 302 def get_letter_html(client): - res = client.get('/loc/letter.html') + res = client.get(letter_url('html')) assert res.status_code == 200 assert res['Content-Type'] == 'text/html; charset=utf-8' return res.content.decode('utf-8') @@ -100,21 +113,21 @@ def test_letter_html_includes_expected_content(client): def test_example_html_works(client): - res = client.get('/loc/example.html') + res = client.get(example_url('html')) assert res.status_code == 200 assert res['Content-Type'] == 'text/html; charset=utf-8' @requires_pdf_rendering def test_letter_pdf_works(admin_client): - res = admin_client.get('/loc/letter.pdf') + res = admin_client.get(letter_url('pdf')) assert res.status_code == 200 assert res['Content-Type'] == 'application/pdf' @requires_pdf_rendering def test_example_pdf_works(client): - res = client.get('/loc/example.pdf') + res = client.get(example_url('pdf')) assert res.status_code == 200 assert res['Content-Type'] == 'application/pdf' @@ -122,21 +135,14 @@ def test_example_pdf_works(client): @requires_pdf_rendering def test_admin_letter_pdf_works(outreach_client): user = UserFactory() - res = outreach_client.get(f'/loc/admin/{user.pk}/letter.pdf') + res = outreach_client.get(admin_letter_url(user.pk)) assert res.status_code == 200 assert res['Content-Type'] == 'application/pdf' -def test_admin_letter_pdf_fails_for_nonexistent_users(admin_client): - res = admin_client.get(f'/loc/admin/1024/letter.pdf') - - # Unfortunately, right now 404's returned by non-i18n routes appear to - # be converted to redirects to locale-prefixed routes. This is annoying - # but it's ultimately okay since it will result in a 404 on the - # locale-prefixed route. We won't test that part because it will make - # the test take longer, though. - assert res.status_code == 302 - assert res.url == "/en/loc/admin/1024/letter.pdf" +def test_admin_letter_pdf_returns_404_for_nonexistent_users(admin_client): + res = admin_client.get(admin_letter_url(1024)) + assert res.status_code == 404 @pytest.mark.django_db @@ -145,10 +151,11 @@ def test_admin_letter_pdf_is_inaccessible_to_non_staff_users(client): client.force_login(user) # Yes, even the user's own LoC should be forbidden to them. - res = client.get(f'/loc/admin/{user.pk}/letter.pdf') + url = admin_letter_url(user.pk) + res = client.get(url) assert res.status_code == 302 - assert res.url == f"/login?next=/loc/admin/{user.pk}/letter.pdf" + assert res.url == f"/login?next={url}" @pytest.mark.django_db @@ -156,17 +163,17 @@ def test_admin_envelopes_pdf_is_inaccessible_to_non_staff_users(client): user = UserFactory() client.force_login(user) - res = client.get(f'/loc/admin/envelopes.pdf') + res = client.get(reverse('loc_envelopes')) assert res.status_code == 302 - assert res.url == f"/login?next=/loc/admin/envelopes.pdf" + assert res.url == f"/login?next={reverse('loc_envelopes')}" @requires_pdf_rendering def test_admin_envelopes_pdf_works(outreach_client): user = create_user_with_all_info() bare_user = UserFactory(phone_number='6141234567', username='blah') - res = outreach_client.get(f'/loc/admin/envelopes.pdf?user_ids={user.pk},{bare_user.pk},zz') + res = outreach_client.get(f'{reverse("loc_envelopes")}?user_ids={user.pk},{bare_user.pk},zz') assert res.status_code == 200 assert res['Content-Type'] == 'application/pdf' assert res.context['users'] == [user] diff --git a/loc/urls.py b/loc/urls.py index 0c28f0f91..28bb965f7 100644 --- a/loc/urls.py +++ b/loc/urls.py @@ -8,5 +8,5 @@ path(r'admin/envelopes.pdf', views.envelopes, name='loc_envelopes'), path(r'admin//letter.pdf', views.letter_of_complaint_pdf_for_user, name='loc_for_user'), - re_path(r'^example\.(html|pdf)$', views.example_doc), + re_path(r'^example\.(html|pdf)$', views.example_doc, name='loc_example'), ] diff --git a/project/justfix_environment.py b/project/justfix_environment.py index df2f68b2b..ff73f1bcc 100644 --- a/project/justfix_environment.py +++ b/project/justfix_environment.py @@ -166,6 +166,9 @@ class JustfixEnvironment(typed_environ.BaseEnvironment): # not provided, mapbox integration will be disabled. MAPBOX_ACCESS_TOKEN: str = '' + # Whether or not to enable internationalization/localization. + ENABLE_I18N: bool = False + class JustfixDevelopmentDefaults(JustfixEnvironment): ''' diff --git a/project/settings.py b/project/settings.py index bfa4512c6..b328a8bb1 100644 --- a/project/settings.py +++ b/project/settings.py @@ -178,7 +178,7 @@ # Internationalization # https://docs.djangoproject.com/en/2.1/topics/i18n/ -LANGUAGE_CODE = 'en-us' +LANGUAGE_CODE = 'en' LANGUAGES = [ ('en', 'English'), @@ -186,9 +186,9 @@ TIME_ZONE = 'UTC' -USE_I18N = True +USE_I18N = env.ENABLE_I18N -USE_L10N = True +USE_L10N = env.ENABLE_I18N USE_TZ = True diff --git a/project/tests/test_views.py b/project/tests/test_views.py index b9a02eecd..1f412c522 100644 --- a/project/tests/test_views.py +++ b/project/tests/test_views.py @@ -15,6 +15,13 @@ from frontend.tests import test_safe_mode +def react_url(path: str) -> str: + base_url = reverse('react') + if base_url.endswith('/'): + base_url = base_url[:-1] + return f"{base_url}{path}" + + def test_get_legacy_form_submission_raises_errors(graphql_client): request = graphql_client.request with pytest.raises(LegacyFormSubmissionError, match='No GraphQL query found'): @@ -45,7 +52,7 @@ def test_get_initial_session_works(graphql_client): def test_invalid_post_returns_400(client): - response = client.post('/en/') + response = client.post(react_url('/')) assert response.status_code == 400 assert response.content == b'No GraphQL query found' @@ -56,7 +63,7 @@ def test_invalid_post_returns_400(client): def test_index_works_when_not_in_safe_mode(client): - response = client.get('/en/') + response = client.get(react_url('/')) assert response.status_code == 200 assert 'JustFix.nyc' in response.context['title_tag'] assert ' Date: Thu, 7 Mar 2019 10:24:10 -0500 Subject: [PATCH 15/20] Enable I18N in CircleCI. --- .circleci/config.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 6c857e45b..25b3dc430 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -9,6 +9,7 @@ jobs: DEBUG: yup ENABLE_WEBPACK_CONTENT_HASH: yup ENABLE_FINDHELP: yup + ENABLE_I18N: yup SPATIALITE_LIBRARY_PATH: mod_spatialite CC_TEST_REPORTER_ID: 0b47f78787493d017e97f3f141ab138e9188d1ebbe149bb0f28a8ff3314dfdd7 steps: From 2ad45161e8d8e5de86e95dc8418f92db748cde71 Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Thu, 7 Mar 2019 10:41:17 -0500 Subject: [PATCH 16/20] Internationalize the graphql endpoint. --- project/tests/test_site_util.py | 7 ++++++- project/urls.py | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/project/tests/test_site_util.py b/project/tests/test_site_util.py index bb4dc969f..5c7bc7562 100644 --- a/project/tests/test_site_util.py +++ b/project/tests/test_site_util.py @@ -1,4 +1,5 @@ from django.test import override_settings, TestCase +from django.conf import settings from ..util.site_util import absolute_reverse, absolutify_url @@ -6,9 +7,13 @@ class SiteUtilsTests(TestCase): @override_settings(DEBUG=False) def test_absolute_reverse_works(self): + if settings.USE_I18N: + url = 'https://example.com/en/graphql' + else: + url = 'https://example.com/graphql' self.assertEqual( absolute_reverse('batch-graphql'), - 'https://example.com/graphql' + url ) def test_absolutify_url_raises_error_on_non_absolute_paths(self): diff --git a/project/urls.py b/project/urls.py index 5059674a0..5e9ac80ee 100644 --- a/project/urls.py +++ b/project/urls.py @@ -40,7 +40,6 @@ path('safe-mode/', include('frontend.safe_mode')), path('legacy-app', redirect_to_legacy_app, name='redirect-to-legacy-app'), path('favicon.ico', redirect_favicon), - path('graphql', GraphQLView.as_view(batch=True), name='batch-graphql'), path('dev/', include(dev_patterns, namespace='dev')), ] @@ -54,5 +53,6 @@ urlpatterns += i18n_patterns( path('loc/', include('loc.urls')), path('hp/', include('hpaction.urls')), + path('graphql', GraphQLView.as_view(batch=True), name='batch-graphql'), re_path(r'.*$', react_rendered_view, name='react'), ) From 66a104596513beb13c0185f42783159e9ebd8f1d Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Thu, 7 Mar 2019 11:40:14 -0500 Subject: [PATCH 17/20] Ensure letter of complaint is always in English. --- loc/views.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/loc/views.py b/loc/views.py index 387d997cb..79a027b62 100644 --- a/loc/views.py +++ b/loc/views.py @@ -6,6 +6,7 @@ from django.contrib.auth.decorators import login_required, permission_required from django.template.loader import render_to_string from django.shortcuts import get_object_or_404 +from django.utils import translation from twofactor.decorators import twofactor_required from users.models import JustfixUser, VIEW_LETTER_REQUEST_PERMISSION @@ -155,6 +156,14 @@ def render_document(request, template_name: str, context: Dict[str, Any], format if format not in ['html', 'pdf']: raise ValueError(f'unknown format "{format}"') + # For now, we always want to localize the letter of complaint in English. + # Even if we don't translate the letter itself to other languages, some + # templating functionality provided by Django (such as date formatting) will + # take the current locale into account, and we don't want e.g. a letter to + # have English paragraphs but Spanish dates. So we'll explicitly set + # the locale here. + translation.activate('en') + if format == 'html': html = render_to_string(template_name, context={ **context, From 3cb6ed0f5354f02a1aee19093b6bfe81490201ea Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Thu, 7 Mar 2019 12:00:18 -0500 Subject: [PATCH 18/20] Remove duplicate locale-redirect code from the JS side. --- frontend/lib/app.tsx | 5 ++--- frontend/lib/routes.ts | 21 +++------------------ frontend/lib/tests/routes.test.ts | 13 ------------- 3 files changed, 5 insertions(+), 34 deletions(-) diff --git a/frontend/lib/app.tsx b/frontend/lib/app.tsx index 2702cd023..42d084e81 100644 --- a/frontend/lib/app.tsx +++ b/frontend/lib/app.tsx @@ -1,7 +1,7 @@ import React, { RefObject } from 'react'; import ReactDOM from 'react-dom'; import autobind from 'autobind-decorator'; -import { BrowserRouter, Switch, Route, RouteComponentProps, withRouter, Redirect } from 'react-router-dom'; +import { BrowserRouter, Switch, Route, RouteComponentProps, withRouter } from 'react-router-dom'; import Loadable from 'react-loadable'; import GraphQlClient from './graphql-client'; @@ -217,8 +217,7 @@ export class AppWithoutRouter extends React.Component : NotFound(props); + return NotFound(props); } render() { diff --git a/frontend/lib/routes.ts b/frontend/lib/routes.ts index e475a4d56..c249b1a48 100644 --- a/frontend/lib/routes.ts +++ b/frontend/lib/routes.ts @@ -1,6 +1,6 @@ import { matchPath, RouteComponentProps } from 'react-router-dom'; import { OnboardingInfoSignupIntent } from './queries/globalTypes'; -import i18n, { I18n } from './i18n'; +import i18n from './i18n'; /** * Metadata about signup intents. @@ -217,7 +217,7 @@ export class RouteMap { private parameterizedRoutes: string[] = []; private isInitialized = false; - constructor(private readonly routes: any, private readonly i18n: I18n = new I18n()) { + constructor(private readonly routes: any) { } private ensureIsInitialized() { @@ -278,21 +278,6 @@ export class RouteMap { } return false; } - - /** - * If the given concrete pathname doesn't exist, see if there's - * a nearby pathname that *does* match, which we can reasonably - * redirect the user to. - */ - getNearestRedirect(pathname: string): string|null { - if (this.i18n.isInitialized && this.i18n.localePathPrefix) { - const prefixWithPath = this.i18n.localePathPrefix + pathname; - if (this.exists(prefixWithPath)) { - return prefixWithPath; - } - } - return null; - } } -export const routeMap = new RouteMap(Routes, i18n); +export const routeMap = new RouteMap(Routes); diff --git a/frontend/lib/tests/routes.test.ts b/frontend/lib/tests/routes.test.ts index 880e91daf..5c577077e 100644 --- a/frontend/lib/tests/routes.test.ts +++ b/frontend/lib/tests/routes.test.ts @@ -1,6 +1,5 @@ import { isModalRoute, RouteMap, getSignupIntentOnboardingInfo } from "../routes"; import { OnboardingInfoSignupIntent } from "../queries/globalTypes"; -import { I18n } from "../i18n"; test('isModalRoute() works', () => { expect(isModalRoute('/blah')).toBe(false); @@ -48,16 +47,4 @@ describe('RouteMap', () => { expect(map.exists('/blah/7')).toBe(true); expect(map.exists('/blah/9/zorp')).toBe(false); }); - - describe('getNearestRedirect()', () => { - it('returns null if no redirect is found', () => { - const map = new RouteMap({ blah: '/blah' }); - expect(map.getNearestRedirect('/zzz')).toBeNull(); - }); - - it('returns localized redirects', () => { - const map = new RouteMap({ blah: '/en/blah' }, new I18n('en')); - expect(map.getNearestRedirect('/blah')).toBe('/en/blah'); - }); - }); }); From 5e434197e66e619074ca28c65fea6b6b72a8cfe2 Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Thu, 7 Mar 2019 17:41:32 -0500 Subject: [PATCH 19/20] Allow I18n to be re-initialized, add event listeners. --- frontend/lambda/tests/lambda.test.tsx | 3 --- frontend/lib/i18n.ts | 29 ++++++++++++++++++--------- frontend/lib/routes.ts | 2 ++ frontend/lib/tests/i18n.test.ts | 20 ++++++++++++++++-- frontend/lib/tests/routes.test.ts | 11 +++++++++- 5 files changed, 49 insertions(+), 16 deletions(-) diff --git a/frontend/lambda/tests/lambda.test.tsx b/frontend/lambda/tests/lambda.test.tsx index 160e10ef1..17e89d79d 100644 --- a/frontend/lambda/tests/lambda.test.tsx +++ b/frontend/lambda/tests/lambda.test.tsx @@ -5,7 +5,6 @@ import { Readable } from 'stream'; import { errorCatchingHandler, handleFromJSONStream, LambdaResponse, isPlainJsObject, getBundleFiles } from '../lambda'; import { AppProps } from '../../lib/app'; import { FakeServerInfo, FakeSessionInfo } from '../../lib/tests/util'; -import i18n from '../../lib/i18n'; const fakeAppProps: AppProps = { initialURL: '/', @@ -14,8 +13,6 @@ const fakeAppProps: AppProps = { initialSession: FakeSessionInfo }; -beforeEach(() => i18n.resetForTesting()); - test('lambda works', async () => { jest.setTimeout(10000); const response = await errorCatchingHandler(fakeAppProps); diff --git a/frontend/lib/i18n.ts b/frontend/lib/i18n.ts index 6d729f22b..c4a775e3c 100644 --- a/frontend/lib/i18n.ts +++ b/frontend/lib/i18n.ts @@ -4,9 +4,13 @@ * Instances start out uninitialized, and must be explicitly * initialized before any other methods or properties can be * accessed. + * + * Once initialized, an instance can actually be re-initialized; + * clients can register to be notified if and when this happens. */ export class I18n { private _locale: null|string = null; + private _changeListeners: Function[] = []; /** * Create an instance, optionally auto-initializing it. @@ -55,10 +59,8 @@ export class I18n { * disabled, or an ISO 639-1 code such as 'en' or 'es'. */ initialize(locale: string) { - if (this._locale !== null) { - throw new Error('i18n is already initialized!'); - } this._locale = locale; + this._changeListeners.forEach(cb => cb()); } /** Return whether the instance is initialized. */ @@ -67,13 +69,20 @@ export class I18n { } /** - * Reset the instance, reverting it to an uninitialized - * state. - * - * As the name implies, this should ONLY be used for testing. + * Register a listener to be notified when the instance + * is initialized or re-initialized. */ - resetForTesting(): void { - this._locale = null; + addChangeListener(cb: Function) { + this._changeListeners.push(cb); + } + + /** Unregister a previously-registered listener. */ + removeChangeListener(cb: Function) { + const index = this._changeListeners.indexOf(cb); + if (index === -1) { + throw new Error('change listener does not exist!'); + } + this._changeListeners.splice(index, 1); } } @@ -87,7 +96,7 @@ export class I18n { * such that an I18n object is passed into it, rather than * grabbing this singleton directly. This will make it easier * to unit test, as well as to eventually get rid of the global - * singleton. + * singleton altogether. */ const i18n = new I18n(); diff --git a/frontend/lib/routes.ts b/frontend/lib/routes.ts index c249b1a48..d21506476 100644 --- a/frontend/lib/routes.ts +++ b/frontend/lib/routes.ts @@ -148,6 +148,8 @@ function createLocalizedRouteInfo(prefix: string) { let currentLocaleRoutes: LocalizedRouteInfo|null = null; +i18n.addChangeListener(() => { currentLocaleRoutes = null; }); + /** * This is an ad-hoc structure that defines URL routes for our app. */ diff --git a/frontend/lib/tests/i18n.test.ts b/frontend/lib/tests/i18n.test.ts index 428f068b7..34d06566d 100644 --- a/frontend/lib/tests/i18n.test.ts +++ b/frontend/lib/tests/i18n.test.ts @@ -18,8 +18,24 @@ describe('I18n', () => { expect(i18n.isInitialized).toBe(true); }); - it('raises an error if initialized twice', () => { - expect(() => new I18n('en').initialize('es')).toThrow(/already initialized/i); + it('raises error if nonexistent listener is removed', () => { + expect(() => new I18n().removeChangeListener(() => {})) + .toThrow(/change listener does not exist/i); + }); + + it('notifies listeners on initialization', () => { + let calls = 0; + const i18n = new I18n(); + const cb = () => calls++; + i18n.addChangeListener(cb); + expect(calls).toBe(0); + i18n.initialize('en'); + expect(calls).toBe(1); + i18n.initialize('es'); + expect(calls).toBe(2); + i18n.removeChangeListener(cb); + i18n.initialize('de'); + expect(calls).toBe(2); }); it('returns locale path prefixes', () => { diff --git a/frontend/lib/tests/routes.test.ts b/frontend/lib/tests/routes.test.ts index 5c577077e..f2c5b9650 100644 --- a/frontend/lib/tests/routes.test.ts +++ b/frontend/lib/tests/routes.test.ts @@ -1,5 +1,14 @@ -import { isModalRoute, RouteMap, getSignupIntentOnboardingInfo } from "../routes"; +import Routes, { isModalRoute, RouteMap, getSignupIntentOnboardingInfo } from "../routes"; import { OnboardingInfoSignupIntent } from "../queries/globalTypes"; +import i18n from "../i18n"; + +test('Routes object responds to locale changes', () => { + expect(Routes.locale.home).toBe('/'); + i18n.initialize('en'); + expect(Routes.locale.home).toBe('/en/'); + i18n.initialize(''); + expect(Routes.locale.home).toBe('/'); +}); test('isModalRoute() works', () => { expect(isModalRoute('/blah')).toBe(false); From da99e82962a09d95ca99609d0b5837804d2342c6 Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Thu, 7 Mar 2019 18:02:02 -0500 Subject: [PATCH 20/20] Add a strip_locale() test helper. --- airtable/tests/test_record.py | 6 +++--- hpaction/tests/test_models.py | 6 +++--- loc/tests/test_admin.py | 4 +++- project/tests/util.py | 22 ++++++++++++++++++++++ 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/airtable/tests/test_record.py b/airtable/tests/test_record.py index 122e5b41e..d26425255 100644 --- a/airtable/tests/test_record.py +++ b/airtable/tests/test_record.py @@ -3,6 +3,7 @@ from users.tests.factories import UserFactory from onboarding.tests.factories import OnboardingInfoFactory +from project.tests.util import strip_locale from loc.tests.factories import LetterRequestFactory, LandlordDetailsFactory from airtable.record import Fields @@ -48,9 +49,8 @@ def test_from_user_works_with_letter_request(): fields = Fields.from_user(lr.user) assert fields.letter_request__will_we_mail is True assert fields.letter_request__created_at == datetime.datetime.utcnow().date().isoformat() - url = fields.letter_request__admin_pdf_url - assert url.startswith('https://example.com/') - assert url.endswith(f'/loc/admin/{lr.user.pk}/letter.pdf') + assert strip_locale(fields.letter_request__admin_pdf_url) == \ + f'https://example.com/loc/admin/{lr.user.pk}/letter.pdf' @pytest.mark.django_db diff --git a/hpaction/tests/test_models.py b/hpaction/tests/test_models.py index 665ee7170..e33e50d50 100644 --- a/hpaction/tests/test_models.py +++ b/hpaction/tests/test_models.py @@ -2,6 +2,7 @@ from freezegun import freeze_time from users.tests.factories import UserFactory +from project.tests.util import strip_locale from .factories import HPActionDocumentsFactory, UploadTokenFactory from ..models import ( HPActionDocuments, UploadToken, UPLOAD_TOKEN_LIFETIME, @@ -41,9 +42,8 @@ def test_create_documents_from_works(self, db, django_file_storage): def test_get_upload_url_works(self, db): token = UploadToken(id='boop') - url = token.get_upload_url() - assert url.startswith('https://example.com/') - assert url.endswith('/hp/upload/boop') + url = strip_locale(token.get_upload_url()) + assert url == 'https://example.com/hp/upload/boop' class TestHPActionDocuments: diff --git a/loc/tests/test_admin.py b/loc/tests/test_admin.py index dbd04528b..32313cdf4 100644 --- a/loc/tests/test_admin.py +++ b/loc/tests/test_admin.py @@ -2,6 +2,7 @@ from users.models import JustfixUser from users.tests.factories import UserFactory +from project.tests.util import strip_locale from loc.admin import LetterRequestInline, print_loc_envelopes from loc.models import LetterRequest @@ -25,4 +26,5 @@ def test_loc_actions_shows_pdf_link_when_user_has_letter_request(): def test_print_loc_envelopes_works(): user = UserFactory() redirect = print_loc_envelopes(None, None, JustfixUser.objects.all()) - assert redirect.url.endswith(f'/loc/admin/envelopes.pdf?user_ids={user.pk}') + url = strip_locale(redirect.url) + assert url == f'/loc/admin/envelopes.pdf?user_ids={user.pk}' diff --git a/project/tests/util.py b/project/tests/util.py index 137d352af..0a78adbe1 100644 --- a/project/tests/util.py +++ b/project/tests/util.py @@ -1,6 +1,7 @@ from unittest.mock import patch from functools import wraps from django.http import QueryDict +from django.conf import settings from project.views import FRONTEND_QUERY_DIR @@ -42,3 +43,24 @@ def wrapper(*fn_args, **fn_kwargs): return wrapper return decorator + + +def strip_locale(url: str) -> str: + ''' + If the given URL has a locale prefix in its + pathname, remove it. For example: + + >>> strip_locale('https://blah/blarg') + 'https://blah/blarg' + + >>> strip_locale('https://blah/en/blarg') + 'https://blah/blarg' + ''' + + # This isn't particularly precise, but it gets + # the job done for testing, which is all we're + # using it for. + for lang, _ in settings.LANGUAGES: + url = url.replace(f"/{lang}/", "/") + + return url