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
7 changes: 7 additions & 0 deletions public/locales/en/enrolment.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@
"labelIsSharingDataAccepted": "I agree to share my information with the event organizer",
"labelLanguage": "Language",
"labelNotificationLanguage": "Language used in notifications",
"culturalRoute": {
"labelIsPartOf": "The visit is part of the cultural route",
"labelNoOrUnknown": "No / I don't know",
"labelYes": "Yes",
"helperText": "Cultural routes are implementation proposals for the City of Helsinki's cultural education plan from early childhood education to upper secondary education. <a>Read more</a>",
"readMoreUrl": "/en/cms-page/cultural-education/cultural-routes"
},
"person": {
"labelEmailAddress": "Email address",
"labelName": "Name",
Expand Down
7 changes: 7 additions & 0 deletions public/locales/fi/enrolment.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@
"labelIsSharingDataAccepted": "Hyväksyn tietojeni jakamisen tapahtuman järjestäjän kanssa",
"labelLanguage": "Kieli",
"labelNotificationLanguage": "Ilmoitusten kieli",
"culturalRoute": {
"labelIsPartOf": "Käynti sisältyy kulttuuripolkuun",
"labelNoOrUnknown": "Ei / en tiedä",
"labelYes": "Kyllä",
"helperText": "Kulttuuripolut ovat Helsingin kaupungin kulttuurikasvatussuunnitelman toteutusehdotuksia varhaiskasvatuksesta toiselle asteelle. <a>Lue lisää kulttuuripoluista</a>",
"readMoreUrl": "/fi/cms-page/kulttuurikasvatus/kulttuuripolulla-kulttuuria-kaikille"
},
"person": {
"labelEmailAddress": "Sähköpostiosoite",
"labelName": "Nimi",
Expand Down
7 changes: 7 additions & 0 deletions public/locales/sv/enrolment.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@
"labelIsSharingDataAccepted": "Jag samtycker till att dela mina uppgifter med arrangören av evenemanget.",
"labelLanguage": "Språk",
"labelNotificationLanguage": "Språket för meddelanden",
"culturalRoute": {
"labelIsPartOf": "Besöket ingår i kulturstigen",
"labelNoOrUnknown": "Nej / jag vet inte",
"labelYes": "Ja",
"helperText": "Kulturstigarna är förslag på genomförande av Helsingfors stads kulturfostransplan från småbarnspedagogik till gymnasieutbildning. <a>Läs mer om kulturstigarna</a>",
"readMoreUrl": "/sv/cms-page/kulturpedagogik/kulturstigen-kultur-for-alla"
},
"person": {
"labelEmailAddress": "E-post",
"labelName": "Namn",
Expand Down
13 changes: 12 additions & 1 deletion src/common/components/formikPersist/FormikPersist.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { FormikProps, useFormikContext } from 'formik';
import cloneDeep from 'lodash/cloneDeep';
import get from 'lodash/get';
import isEqual from 'lodash/isEqual';
import set from 'lodash/set';
import * as React from 'react';

import useIsMounted from '../../../hooks/useIsMounted';
Expand All @@ -26,13 +29,16 @@ export interface PersistProps {
debounceTime?: number;
isSessionStorage?: boolean;
initialValues: Record<string, unknown>;
// Dot-notation paths of fields that are restored using initial values:
alwaysFreshFields?: string[];
}

const FormikPersist = ({
debounceTime = 300,
isSessionStorage = false,
name,
initialValues,
alwaysFreshFields = [],
}: PersistProps): null => {
const isMounted = useIsMounted();
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -78,7 +84,12 @@ const FormikPersist = ({
storedFormikStateObject &&
objectStructureMatches(initialValues, storedFormikStateObject.values)
) {
formik.setValues(storedFormikStateObject.values, true);
const valuesToRestore = cloneDeep(storedFormikStateObject.values);
// Restore fresh fields with their initial values instead of stored ones:
for (const path of alwaysFreshFields) {
set(valuesToRestore, path, get(initialValues, path));
}
formik.setValues(valuesToRestore, true);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,56 @@ test('attempts to rehydrate on mount if session storage is true on props', async
);
});
});

test('alwaysFreshFields resets listed fields to their initial values', async () => {
let injected: FormikProps<{ regularField: string; freshField: string }>;

(localStorage.getItem as jest.Mock).mockReturnValueOnce(
JSON.stringify({
...defaultState,
values: {
regularField: 'Persisted value of regular field',
freshField: 'Persisted value of fresh field',
},
})
);

render(
<Formik
initialValues={{
regularField: 'Initial value of regular field in Formik',
freshField: 'Initial value of fresh field in Formik',
}}
onSubmit={jest.fn()}
>
{(props: FormikProps<{ regularField: string; freshField: string }>) => {
injected = props;

return (
<div>
<Persist
name={formName}
debounceTime={0}
initialValues={{
regularField: 'Initial value of regular field in Persist',
freshField: 'Initial value of fresh field in Persist',
}}
alwaysFreshFields={['freshField']}
/>
</div>
);
}}
</Formik>
);

expect(localStorage.getItem).toHaveBeenCalled();

// regularField is not in alwaysFreshFields, so it should be restored from storage
expect(injected!.values.regularField).toEqual(
'Persisted value of regular field'
);
// freshField is in alwaysFreshFields, so it must be reset to its initial value:
expect(injected!.values.freshField).toEqual(
'Initial value of fresh field in Persist'
);
});
27 changes: 27 additions & 0 deletions src/domain/enrolment/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { IsPartOfCulturalRoute } from '../enrolmentForm/constants';
import { validIsPartOfCulturalRouteToBoolean } from '../utils';

describe('validIsPartOfCulturalRouteToBoolean', () => {
test.each<IsPartOfCulturalRoute | string>([
IsPartOfCulturalRoute.YES, // Test that the enum version works
'YES', // Test that the string version also works
])('validIsPartOfCulturalRouteToBoolean(%p) returns true', (input) => {
expect(validIsPartOfCulturalRouteToBoolean(input)).toBe(true);
});

test.each<IsPartOfCulturalRoute | string>([
IsPartOfCulturalRoute.NO_OR_UNKNOWN, // Test that the enum version works
'NO_OR_UNKNOWN', // Test that the string version also works
])('validIsPartOfCulturalRouteToBoolean(%p) returns false', (input) => {
expect(validIsPartOfCulturalRouteToBoolean(input)).toBe(false);
});

test.each(['', ' ', 'yes', 'NO', 'unanswered', null, undefined, true, 0, 1])(
'validIsPartOfCulturalRouteToBoolean(%p) throws on invalid value',
(input) => {
expect(() => validIsPartOfCulturalRouteToBoolean(input)).toThrow(
`Invalid isPartOfCulturalRoute value ${input}`
);
}
);
});
6 changes: 6 additions & 0 deletions src/domain/enrolment/enrolmentForm/EnrolmentForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
nameToLabelPath,
} from './constants';
import styles from './enrolmentForm.module.scss';
import IsPartOfCulturalRouteField from './IsPartOfCulturalRouteField';
import getValidationSchema from './ValidationSchema';
import ErrorMessage from '../../../common/components/form/ErrorMessage';
import CheckboxField from '../../../common/components/form/fields/CheckboxField';
Expand Down Expand Up @@ -103,6 +104,7 @@ const EnrolmentForm: React.FC<Props> = ({
<FormikPersist
name={FORM_NAMES.ENROLMENT_FORM}
initialValues={initialValues}
alwaysFreshFields={['isPartOfCulturalRoute']}
/>
<h2>{t('enrolment:enrolmentForm.studyGroup.titleNotifier')}</h2>
<FormErrorNotification
Expand Down Expand Up @@ -206,6 +208,10 @@ const EnrolmentForm: React.FC<Props> = ({
</FormGroup>
</div>

<FormGroup>
<IsPartOfCulturalRouteField formikFieldName="isPartOfCulturalRoute" />
</FormGroup>
Comment on lines +211 to +213

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.

I don't think it changes the result or analysis any how, but now I could understand this is "No / I don't know what cultural route is / I don't want to give an answer".

I would have personally used just a checkbox. If I know what a cultural route is, I would check it. I'm enrolling and I need to check that all the information that I have given with form is valid. If we would really trust in users and in the given input, this would be easy boolean analysis and way easier to implement and even cleaner UI and UX. Now we are doing much work only to guide the report analysis, because the input would still be the same -- This implementation still does not answer to report analysis question whether or not the user has seen this question.

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.

I talked with the product owner about their needs and they wanted this so that people always choose a value, that they need to make a choice. So, making the user always choose a value for this is intentional and as per product owner's wishes.


<div className={styles.rowWith2Columns}>
<div>
<h2>{t('enrolment:enrolmentForm.studyGroup.titleGroup')}</h2>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { Link } from '@city-of-helsinki/react-helsinki-headless-cms';
import { useField } from 'formik';
import { RadioButton, SelectionGroup } from 'hds-react';
import { Trans, useTranslation } from 'next-i18next';
import React from 'react';

import { IsPartOfCulturalRoute, nameToLabelPath } from './constants';
import type { I18nNamespace } from '../../../types';

type Props = {
formikFieldName: string;
};

/**
* React component for "Is part of cultural route?" field in the enrolment form,
* with choices of "No / I don't know" and "Yes".
*/
const IsPartOfCulturalRouteField: React.FC<Props> = ({ formikFieldName }) => {
const { t } = useTranslation<I18nNamespace>();
const [field, meta] = useField(formikFieldName);
const errorText = meta.touched && meta.error ? t(meta.error) : undefined;
const readMoreUrl = t('enrolment:enrolmentForm.culturalRoute.readMoreUrl');

return (
<SelectionGroup
label={t(nameToLabelPath['isPartOfCulturalRoute'])}
direction="horizontal"
required
aria-required
errorText={errorText}
/* @ts-expect-error TS2322 SelectionGroup's types allow only string, but Trans works */
helperText={
<Trans
t={t}
i18nKey={'enrolment:enrolmentForm.culturalRoute.helperText'}
components={{ a: <Link href={readMoreUrl} /> }}
/>
}
>
<RadioButton
id="isPartOfCulturalRoute-noOrUnknown"
name={field.name}
value={IsPartOfCulturalRoute.NO_OR_UNKNOWN}
label={t('enrolment:enrolmentForm.culturalRoute.labelNoOrUnknown')}
checked={field.value === IsPartOfCulturalRoute.NO_OR_UNKNOWN}
onChange={field.onChange}
onBlur={field.onBlur}
/>
<RadioButton
id="isPartOfCulturalRoute-yes"
name={field.name}
value={IsPartOfCulturalRoute.YES}
label={t('enrolment:enrolmentForm.culturalRoute.labelYes')}
checked={field.value === IsPartOfCulturalRoute.YES}
onChange={field.onChange}
onBlur={field.onBlur}
/>
</SelectionGroup>
);
};

export default IsPartOfCulturalRouteField;
7 changes: 7 additions & 0 deletions src/domain/enrolment/enrolmentForm/ValidationSchema.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as Yup from 'yup';

import { IsPartOfCulturalRoute } from './constants';
import { VALIDATION_MESSAGE_KEYS } from '../../app/forms/constants';

type EnrolmentFormValidationSchemaProps = {
Expand All @@ -14,6 +15,12 @@ export default function getValidationSchema({
isQueueEnrolment,
}: EnrolmentFormValidationSchemaProps) {
return Yup.object().shape({
isPartOfCulturalRoute: Yup.string()
.oneOf(
[IsPartOfCulturalRoute.NO_OR_UNKNOWN, IsPartOfCulturalRoute.YES],
VALIDATION_MESSAGE_KEYS.STRING_REQUIRED
)
.required(VALIDATION_MESSAGE_KEYS.STRING_REQUIRED),
hasEmailNotification: Yup.bool().oneOf(
[true],
'enrolment:enrolmentForm.validation.hasEmailNotification'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ import {
within,
sleep,
} from '../../../../utils/testUtils';
import { defaultEnrolmentInitialValues } from '../constants';
import {
defaultEnrolmentInitialValues,
IsPartOfCulturalRoute,
} from '../constants';
import EnrolmentForm, { Props } from '../EnrolmentForm';

configure({ defaultHidden: true });
Expand Down Expand Up @@ -113,6 +116,14 @@ test('renders form and user can fill it and submit and form is saved to local st

// // close dropdown

// Answer "Is part of cultural route?" question
const culturalRouteGroup = await screen.findByRole('group', {
name: /käynti sisältyy kulttuuripolkuun/i,
});
await userEvent.click(
within(culturalRouteGroup).getByRole('radio', { name: /kyllä/i })
);

await userEvent.click(screen.getByRole('combobox', { name: /luokka-aste/i }));

await userEvent.type(screen.getByLabelText(/lapsia/i), '10');
Expand Down Expand Up @@ -411,6 +422,7 @@ if (isFeatureEnabled('FORMIK_PERSIST')) {
isSameResponsiblePerson: true,
isSharingDataAccepted: false,
isMandatoryAdditionalInformationRequired: false,
isPartOfCulturalRoute: IsPartOfCulturalRoute.NO_OR_UNKNOWN,
language: '',
studyGroup: {
person: {
Expand Down Expand Up @@ -504,6 +516,23 @@ if (isFeatureEnabled('FORMIK_PERSIST')) {
expect(
screen.getByRole('combobox', { name: /luokka-aste/i })
).toHaveTextContent('1. luokka');

// Test that "Is part of cultural route?" question is not answered,
// because this field should not be restored from local storage:
const culturalRouteGroup = await screen.findByRole('group', {
name: /käynti sisältyy kulttuuripolkuun/i,
});
const isNotPartOfCulturalRoute = within(culturalRouteGroup).getByRole(
'radio',
{ name: /ei \/ en tiedä/i }
);
const isPartOfCulturalRoute = within(culturalRouteGroup).getByRole(
'radio',
{ name: /kyllä/i }
);
expect(isNotPartOfCulturalRoute).not.toBeChecked();
expect(isPartOfCulturalRoute).not.toBeChecked();

const childrenCountInput = screen.getByRole('spinbutton', {
name: /lapsia \*/i,
});
Expand Down
Loading
Loading