Skip to content

Commit

Permalink
refactor(organizations): code cleanup and patch data cleanup in accou…
Browse files Browse the repository at this point in the history
…nt settings and tos view TASK-1292 (#5290)

### 👀 Preview steps
1. ℹ️ have an account and a project
2. Go into account settings view
3. Change information on some field(s)
4. Save the changes
5. Check in the network tab the call to the `/me` endpoint. The payload
should only contain the changed fields.
6. Enable a TOS view by following [this
document](https://www.notion.so/kobotoolbox/Setting-up-TOS-1447e515f65480f8ab34f69d42d806f8)
7. In the TOS view, fill in the information and save (you may want to
`preserve log` before saving data)
8. Check the network tab. Payload should contain only the changed field.

### 💭 Notes
We're refactoring accountDetails and TOS view on the way of implementing
some changes on displayed fields:
- Removed the "global" observer from mob-x which was causing linting
issues in favor of using the `useSession` hook created at
#5303
- Cleaned up the code
- Changed in a way that only changed fields are sent to the `PATCH`
request to update data
  • Loading branch information
pauloamorimbr authored Nov 28, 2024
1 parent 38393e0 commit aaa8aac
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 177 deletions.
41 changes: 2 additions & 39 deletions jsapp/js/account/account.utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import type {AccountFieldsValues} from './account.constants';
import envStore from '../envStore';
import {USER_FIELD_NAMES} from './account.constants';

export function getInitialAccountFieldsValues(): AccountFieldsValues {
return {
Expand All @@ -25,41 +23,6 @@ export function getInitialAccountFieldsValues(): AccountFieldsValues {
* For given field values produces an object to use with the `/me` endpoint for
* updating the `extra_details`.
*/
export function getProfilePatchData(fields: AccountFieldsValues) {
// HACK: dumb down the `output` type here, so TS doesn't have a problem with
// types inside the `forEach` loop below, and the output is compatible with
// functions from `api.ts` file.
const output: {extra_details: {[key: string]: any}} = {
extra_details: getInitialAccountFieldsValues(),
};

// To patch correctly with recent changes to the backend,
// ensure that we send empty strings if the field is left blank.

// We should only overwrite user metadata that the user can see.
// Fields that:
// (a) are enabled in constance
// (b) the frontend knows about

// Make a list of user metadata fields to include in the patch
const presentMetadataFields =
// Fields enabled in constance
envStore.data
.getUserMetadataFieldNames()
// Intersected with:
.filter(
(fieldName) =>
// Fields the frontend knows about
fieldName in USER_FIELD_NAMES
);

// Populate the patch with user form input, or empty strings.
presentMetadataFields.forEach((fieldName) => {
output.extra_details[fieldName] = fields[fieldName] || '';
});

// Always include require_auth, defaults to 'false'.
output.extra_details.require_auth = fields.require_auth ? true : false;

return output;
export function getProfilePatchData(fields: Partial<AccountFieldsValues>) {
return {extra_details: fields};
}
5 changes: 2 additions & 3 deletions jsapp/js/account/accountFieldsEditor.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ interface AccountFieldsEditorProps {
* `displayedFields` prop)
*/
values: AccountFieldsValues;
onChange: (fields: AccountFieldsValues) => void;
onFieldChange: (fieldName: UserFieldName, value: UserFieldValue) => void;
}

/**
Expand Down Expand Up @@ -82,8 +82,7 @@ export default function AccountFieldsEditor(props: AccountFieldsEditorProps) {
fieldName: UserFieldName,
newValue: UserFieldValue
) {
const newValues = {...props.values, [fieldName]: newValue};
props.onChange(newValues);
props.onFieldChange(fieldName, newValue);
}

const cleanedUrl = (value: string) => {
Expand Down
181 changes: 80 additions & 101 deletions jsapp/js/account/accountSettingsRoute.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import React, {useEffect, useState} from 'react';
import type React from 'react';
import {useEffect, useState} from 'react';
import Button from 'js/components/common/button';
import InlineMessage from 'js/components/common/inlineMessage';
import {observer} from 'mobx-react';
import type {Form} from 'react-router-dom';
import {unstable_usePrompt as usePrompt} from 'react-router-dom';
import bem, {makeBem} from 'js/bem';
import sessionStore from 'js/stores/session';
import './accountSettings.scss';
import {notify} from 'js/utils';
import {dataInterface} from '../dataInterface';
Expand All @@ -21,119 +19,93 @@ import type {
AccountFieldsErrors,
} from './account.constants';
import {HELP_ARTICLE_ANON_SUBMISSIONS_URL} from 'js/constants';
import {useSession} from '../stores/useSession';

bem.AccountSettings = makeBem(null, 'account-settings', 'form');
bem.AccountSettings__left = makeBem(bem.AccountSettings, 'left');
bem.AccountSettings__right = makeBem(bem.AccountSettings, 'right');
bem.AccountSettings__item = makeBem(bem.FormModal, 'item');
bem.AccountSettings__actions = makeBem(bem.AccountSettings, 'actions');

interface Form {
isPristine: boolean;
/**
* Whether we have loaded the user metadata values. Used to avoid displaying
* blank form with values coming in a moment later (in visible way).
*/
isUserDataLoaded: boolean;
fields: AccountFieldsValues;
fieldsWithErrors: {
extra_details?: AccountFieldsErrors;
};
}

const AccountSettings = observer(() => {
const [form, setForm] = useState<Form>({
isPristine: true,
isUserDataLoaded: false,
fields: getInitialAccountFieldsValues(),
fieldsWithErrors: {},
});
const AccountSettings = () => {
const [isPristine, setIsPristine] = useState(true);
const [fieldErrors, setFieldErrors] = useState<AccountFieldsErrors>({});
const [formFields, setFormFields] = useState<AccountFieldsValues>(
getInitialAccountFieldsValues()
);
const [editedFields, setEditedFields] = useState<
Partial<AccountFieldsValues>
>({});

const {currentLoggedAccount, refreshAccount} = useSession();

useEffect(() => {
if (
!sessionStore.isPending &&
sessionStore.isInitialLoadComplete &&
!sessionStore.isInitialRoute
) {
sessionStore.refreshAccount();
}
}, []);
useEffect(() => {
const currentAccount = sessionStore.currentAccount;
if (
!sessionStore.isPending &&
sessionStore.isInitialLoadComplete &&
'email' in currentAccount
) {
setForm({
...form,
isUserDataLoaded: true,
fields: {
name: currentAccount.extra_details.name,
organization_type: currentAccount.extra_details.organization_type,
organization: currentAccount.extra_details.organization,
organization_website:
currentAccount.extra_details.organization_website,
sector: currentAccount.extra_details.sector,
gender: currentAccount.extra_details.gender,
bio: currentAccount.extra_details.bio,
city: currentAccount.extra_details.city,
country: currentAccount.extra_details.country,
require_auth: currentAccount.extra_details.require_auth,
twitter: currentAccount.extra_details.twitter,
linkedin: currentAccount.extra_details.linkedin,
instagram: currentAccount.extra_details.instagram,
newsletter_subscription:
currentAccount.extra_details.newsletter_subscription,
},
fieldsWithErrors: {},
});
if (!currentLoggedAccount) {
return;
}
}, [sessionStore.isPending]);

setFormFields({
name: currentLoggedAccount.extra_details.name,
organization_type: currentLoggedAccount.extra_details.organization_type,
organization: currentLoggedAccount.extra_details.organization,
organization_website: currentLoggedAccount.extra_details.organization_website,
sector: currentLoggedAccount.extra_details.sector,
gender: currentLoggedAccount.extra_details.gender,
bio: currentLoggedAccount.extra_details.bio,
city: currentLoggedAccount.extra_details.city,
country: currentLoggedAccount.extra_details.country,
require_auth: currentLoggedAccount.extra_details.require_auth,
twitter: currentLoggedAccount.extra_details.twitter,
linkedin: currentLoggedAccount.extra_details.linkedin,
instagram: currentLoggedAccount.extra_details.instagram,
newsletter_subscription:
currentLoggedAccount.extra_details.newsletter_subscription,
});
}, [currentLoggedAccount]);

usePrompt({
when: !form.isPristine,
when: !isPristine,
message: t('You have unsaved changes. Leave settings without saving?'),
});

const onUpdateComplete = () => {
notify(t('Updated profile successfully'));
setIsPristine(true);
setFieldErrors({});
};

const onUpdateFail = (errors: AccountFieldsErrors) => {
setFieldErrors(errors);
};

const updateProfile = (e: React.FormEvent) => {
e?.preventDefault?.(); // Prevent form submission page reload

const profilePatchData = getProfilePatchData(form.fields);
const patchData = getProfilePatchData(editedFields);
dataInterface
.patchProfile(profilePatchData)
.patchProfile(patchData)
.done(() => {
onUpdateComplete();
refreshAccount();
})
.fail((...args: any) => {
onUpdateFail(args);
onUpdateFail(args[0].responseJSON);
});
};

const onAccountFieldsEditorChange = (fields: AccountFieldsValues) => {
setForm({
...form,
fields: fields,
isPristine: false,
});
};

const onUpdateComplete = () => {
notify(t('Updated profile successfully'));
setForm({
...form,
isPristine: true,
fieldsWithErrors: {},
const onFieldChange = (fieldName: string, value: string | boolean) => {
setFormFields({
...formFields,
[fieldName]: value,
});
};

const onUpdateFail = (data: any) => {
setForm({
...form,
isPristine: false,
fieldsWithErrors: data[0].responseJSON,
setEditedFields({
...editedFields,
[fieldName]: value,
});
setIsPristine(false);
};

const accountName = sessionStore.currentAccount.username;
const accountName = currentLoggedAccount?.username || '';

return (
<bem.AccountSettings onSubmit={updateProfile}>
Expand All @@ -143,49 +115,56 @@ const AccountSettings = observer(() => {
className='account-settings-save'
size={'l'}
isSubmit
label={t('Save Changes') + (form.isPristine ? '' : ' *')}
label={t('Save Changes') + (isPristine ? '' : ' *')}
/>
</bem.AccountSettings__actions>

<bem.AccountSettings__item m={'column'}>
<bem.AccountSettings__item m='username'>
<Avatar size='m' username={accountName} isUsernameVisible/>
<Avatar size='m' username={accountName} isUsernameVisible />
</bem.AccountSettings__item>

{sessionStore.isInitialLoadComplete && form.isUserDataLoaded && (
{currentLoggedAccount && (
<bem.AccountSettings__item m='fields'>
<InlineMessage
type='warning'
icon='information'
message={(
message={
<>
<strong>
{t('You can now control whether to allow anonymous submissions in web forms for each project. Previously, this was an account-wide setting.')}
{t(
'You can now control whether to allow anonymous submissions in web forms for each project. Previously, this was an account-wide setting.'
)}
</strong>
&nbsp;
{t('This privacy feature is now a per-project setting. New projects will require authentication by default.')}
{t(
'This privacy feature is now a per-project setting. New projects will require authentication by default.'
)}
&nbsp;
<a
href={envStore.data.support_url + HELP_ARTICLE_ANON_SUBMISSIONS_URL}
href={
envStore.data.support_url +
HELP_ARTICLE_ANON_SUBMISSIONS_URL
}
target='_blank'
>
{t('Learn more about these changes here.')}
</a>
</>
)}
}
className='anonymous-submission-notice'
/>

<AccountFieldsEditor
errors={form.fieldsWithErrors.extra_details}
values={form.fields}
onChange={onAccountFieldsEditorChange}
errors={fieldErrors}
values={formFields}
onFieldChange={onFieldChange}
/>
</bem.AccountSettings__item>
)}
</bem.AccountSettings__item>
</bem.AccountSettings>
);
});
};

export default AccountSettings;
Loading

0 comments on commit aaa8aac

Please sign in to comment.