From aaa8aac0e44360f490fa98ab660c12ed63a5ffcc Mon Sep 17 00:00:00 2001 From: Paulo Amorim Date: Thu, 28 Nov 2024 12:51:37 -0300 Subject: [PATCH] refactor(organizations): code cleanup and patch data cleanup in account settings and tos view TASK-1292 (#5290) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### 👀 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 https://github.com/kobotoolbox/kpi/pull/5303 - Cleaned up the code - Changed in a way that only changed fields are sent to the `PATCH` request to update data --- jsapp/js/account/account.utils.ts | 41 +--- .../account/accountFieldsEditor.component.tsx | 5 +- jsapp/js/account/accountSettingsRoute.tsx | 181 ++++++++---------- jsapp/js/tos/tosForm.component.tsx | 80 ++++---- 4 files changed, 130 insertions(+), 177 deletions(-) diff --git a/jsapp/js/account/account.utils.ts b/jsapp/js/account/account.utils.ts index 6a242324db..792f0bb6cb 100644 --- a/jsapp/js/account/account.utils.ts +++ b/jsapp/js/account/account.utils.ts @@ -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 { @@ -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) { + return {extra_details: fields}; } diff --git a/jsapp/js/account/accountFieldsEditor.component.tsx b/jsapp/js/account/accountFieldsEditor.component.tsx index b52a1cdcaf..ce8e5dea81 100644 --- a/jsapp/js/account/accountFieldsEditor.component.tsx +++ b/jsapp/js/account/accountFieldsEditor.component.tsx @@ -42,7 +42,7 @@ interface AccountFieldsEditorProps { * `displayedFields` prop) */ values: AccountFieldsValues; - onChange: (fields: AccountFieldsValues) => void; + onFieldChange: (fieldName: UserFieldName, value: UserFieldValue) => void; } /** @@ -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) => { diff --git a/jsapp/js/account/accountSettingsRoute.tsx b/jsapp/js/account/accountSettingsRoute.tsx index 91e7b6ea69..bdaf7368ce 100644 --- a/jsapp/js/account/accountSettingsRoute.tsx +++ b/jsapp/js/account/accountSettingsRoute.tsx @@ -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'; @@ -21,6 +19,7 @@ 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'); @@ -28,112 +27,85 @@ 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
({ - isPristine: true, - isUserDataLoaded: false, - fields: getInitialAccountFieldsValues(), - fieldsWithErrors: {}, - }); +const AccountSettings = () => { + const [isPristine, setIsPristine] = useState(true); + const [fieldErrors, setFieldErrors] = useState({}); + const [formFields, setFormFields] = useState( + getInitialAccountFieldsValues() + ); + const [editedFields, setEditedFields] = useState< + Partial + >({}); + + 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 ( @@ -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 ? '' : ' *')} /> - + - {sessionStore.isInitialLoadComplete && form.isUserDataLoaded && ( + {currentLoggedAccount && ( - {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.' + )}   - {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.' + )}   {t('Learn more about these changes here.')} - )} + } className='anonymous-submission-notice' /> )} ); -}); +}; export default AccountSettings; diff --git a/jsapp/js/tos/tosForm.component.tsx b/jsapp/js/tos/tosForm.component.tsx index 931f8295bb..48213c236a 100644 --- a/jsapp/js/tos/tosForm.component.tsx +++ b/jsapp/js/tos/tosForm.component.tsx @@ -1,7 +1,7 @@ -import React, {useState, useEffect} from 'react'; +import type React from 'react'; +import {useState, useEffect} from 'react'; import Button from 'js/components/common/button'; import envStore from 'js/envStore'; -import sessionStore from 'js/stores/session'; import {fetchGet, fetchPatch, fetchPost, handleApiFail} from 'js/api'; import styles from './tosForm.module.scss'; import type {FailResponse, PaginatedResponse} from 'js/dataInterface'; @@ -16,6 +16,7 @@ import type { AccountFieldsErrors, } from 'js/account/account.constants'; import {currentLang, notify} from 'js/utils'; +import {useSession} from '../stores/useSession'; /** A slug for the `sitewide_messages` endpoint */ const TOS_SLUG = 'terms_of_service'; @@ -51,10 +52,13 @@ export default function TOSForm() { const [announcementMessage, setAnnouncementMessage] = useState< string | undefined >(); - const [fields, setFields] = useState( + const [formFields, setFormFields] = useState( getInitialAccountFieldsValues() ); const [fieldsErrors, setFieldsErrors] = useState({}); + const [editedFields, setEditedFields] = useState< + Partial + >({}); const fieldsToShow = envStore.data.getUserMetadataRequiredFieldNames(); if ( @@ -63,6 +67,8 @@ export default function TOSForm() { fieldsToShow.push('newsletter_subscription'); } + const {currentLoggedAccount, logOut} = useSession(); + // Get TOS message from endpoint useEffect(() => { const getTOS = async () => { @@ -104,30 +110,40 @@ export default function TOSForm() { // (including the non-required ones that will be hidden, but passed to the API // so that they will not get erased). useEffect(() => { - if ('email' in sessionStore.currentAccount) { - const data = sessionStore.currentAccount; - setFields({ - name: data.extra_details.name, - organization: data.extra_details.organization, - organization_website: data.extra_details.organization_website, - organization_type: data.extra_details.organization_type, - sector: data.extra_details.sector, - gender: data.extra_details.gender, - bio: data.extra_details.bio, - city: data.extra_details.city, - country: data.extra_details.country, - require_auth: data.extra_details.require_auth, - twitter: data.extra_details.twitter, - linkedin: data.extra_details.linkedin, - instagram: data.extra_details.instagram, - newsletter_subscription: data.extra_details.newsletter_subscription, - }); + if (!currentLoggedAccount) { + return; } - }, [sessionStore.isAuthStateKnown]); - function onAccountFieldsEditorChange(newFields: AccountFieldsValues) { - setFields(newFields); - } + setFormFields({ + name: currentLoggedAccount.extra_details.name, + organization: currentLoggedAccount.extra_details.organization, + organization_website: + currentLoggedAccount.extra_details.organization_website, + organization_type: currentLoggedAccount.extra_details.organization_type, + 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]); + + const onFieldChange = (fieldName: string, value: string | boolean) => { + setFormFields({ + ...formFields, + [fieldName]: value, + }); + setEditedFields({ + ...editedFields, + [fieldName]: value, + }); + }; /** * Submitting does two things (with two consecutive API calls): @@ -146,7 +162,7 @@ export default function TOSForm() { // them. if (fieldsToShow.length > 0) { // Get data for the user endpoint - const profilePatchData = getProfilePatchData(fields); + const profilePatchData = getProfilePatchData(editedFields); try { await fetchPatch(ME_ENDPOINT, profilePatchData); @@ -183,16 +199,12 @@ export default function TOSForm() { function leaveForm() { setIsFormPending(true); - sessionStore.logOut(); + logOut(); } // We are waiting for few pieces of data: the message, fields definitions from // environment endpoint and fields data from me endpoint - if ( - !announcementMessage || - !envStore.isReady || - !sessionStore.isAuthStateKnown - ) { + if (!announcementMessage || !envStore.isReady || !currentLoggedAccount) { return ; } @@ -217,8 +229,8 @@ export default function TOSForm() { )}