From 233d86e6f3d40cf30eecb4c5b0bdeb4898594c9a Mon Sep 17 00:00:00 2001 From: AlexisG Date: Thu, 7 Nov 2024 15:12:09 +0100 Subject: [PATCH] fix: Creation of a contact when selecting a linked contact When we wanted to create a new contact from the linked contact addition, the form component was rendered because the creation of the new contact triggers useQueries higher up the React tree. This re-rendering caused the form to lose any changes it had already made, such as adding the new linked contact. --- .../ContactForm/RelatedContactList.jsx | 4 +- .../ContactCard/ContactForm/helpers.js | 4 +- .../ContactCard/ContactForm/index.jsx | 30 +++++++- .../ContactCard/ContactForm/index.spec.jsx | 74 ++++++++++++++++++- 4 files changed, 108 insertions(+), 4 deletions(-) diff --git a/src/components/ContactCard/ContactForm/RelatedContactList.jsx b/src/components/ContactCard/ContactForm/RelatedContactList.jsx index 21fbe2d82..b29691cc3 100644 --- a/src/components/ContactCard/ContactForm/RelatedContactList.jsx +++ b/src/components/ContactCard/ContactForm/RelatedContactList.jsx @@ -2,6 +2,7 @@ import PropTypes from 'prop-types' import React from 'react' import { useForm } from 'react-final-form' +import { makeDisplayName } from 'cozy-client/dist/models/contact' import ContactsListModal from 'cozy-ui/transpiled/react/ContactsListModal' export const RelatedContactList = ({ name, onClose, contacts }) => { @@ -11,7 +12,8 @@ export const RelatedContactList = ({ name, onClose, contacts }) => { * @param {import('cozy-client/types/types').IOCozyContact} contact */ const onClickContactsListModal = contact => { - change(name, contact.displayName) + // Use `makeDisplayName` because if the contact is newly created, it has no `displayName` attribute. (Creation of a contact when selecting a linked contact) + change(name, makeDisplayName(contact)) change(`${name}Id`, contact._id) onClose() } diff --git a/src/components/ContactCard/ContactForm/helpers.js b/src/components/ContactCard/ContactForm/helpers.js index b6fb757d0..b105f5faa 100644 --- a/src/components/ContactCard/ContactForm/helpers.js +++ b/src/components/ContactCard/ContactForm/helpers.js @@ -2,6 +2,7 @@ import isEqual from 'lodash/isEqual' import merge from 'lodash/merge' import { Association } from 'cozy-client' +import { makeDisplayName } from 'cozy-client/dist/models/contact' import contactToFormValues from './contactToFormValues' import { DOCTYPE_CONTACTS } from '../../../helpers/doctypes' @@ -203,7 +204,8 @@ export const makeRelatedContact = contact => { } const relatedData = contact.related.data.reduce((acc, curr) => { - acc[curr._id] = curr.displayName + // Use `makeDisplayName` because if the contact is newly created, it has no `displayName` attribute. (Creation of a contact when selecting a linked contact) + acc[curr._id] = curr.displayName || makeDisplayName(curr) return acc }, {}) diff --git a/src/components/ContactCard/ContactForm/index.jsx b/src/components/ContactCard/ContactForm/index.jsx index 1350530de..302da64d6 100644 --- a/src/components/ContactCard/ContactForm/index.jsx +++ b/src/components/ContactCard/ContactForm/index.jsx @@ -4,6 +4,7 @@ import PropTypes from 'prop-types' import React from 'react' import { Form } from 'react-final-form' +import { getHasManyItems } from 'cozy-client/dist/associations/HasMany' import { useI18n } from 'cozy-ui/transpiled/react/providers/I18n' import ContactFormField from './ContactFormField' @@ -104,4 +105,31 @@ ContactForm.propTypes = { }) } -export default ContactForm +// Used to avoid unnecessary multiple rendering of ContactForm when creating a new contact in another way. +// These unnecessary renderings prevented the addition of a newly created linked contact. (Creation of a contact when selecting a linked contact) +export const isSameContactProp = (prevProps, nextProps) => { + if (!prevProps.contact?.relationships || !nextProps.contact?.relationships) { + return false + } + + const prevContactIdsRelated = getHasManyItems( + prevProps.contact, + 'related' + ).map(r => r._id) + const nextContactIdsRelated = getHasManyItems( + nextProps.contact, + 'related' + ).map(r => r._id) + + if ( + prevContactIdsRelated.length !== nextContactIdsRelated.length || + !prevContactIdsRelated.every(id => nextContactIdsRelated.includes(id)) + ) { + return false + } + + return true +} + +// export default ContactForm +export default React.memo(ContactForm, isSameContactProp) diff --git a/src/components/ContactCard/ContactForm/index.spec.jsx b/src/components/ContactCard/ContactForm/index.spec.jsx index 704cf14be..198216f91 100644 --- a/src/components/ContactCard/ContactForm/index.spec.jsx +++ b/src/components/ContactCard/ContactForm/index.spec.jsx @@ -3,7 +3,7 @@ import React from 'react' import { createMockClient } from 'cozy-client' -import ContactForm from './index' +import ContactForm, { isSameContactProp } from './index' import { johnDoeContact as contact } from '../../../helpers/testData' import AppLike from '../../../tests/Applike' @@ -215,4 +215,76 @@ describe('ContactForm', () => { expect(onSubmit).not.toBeCalled() }) + + describe('isSameContactProp', () => { + it('should return true if contacts have the same "related" relationships', () => { + const prevProps = { + contact: { + relationships: { + related: { + data: [{ _id: '1' }, { _id: '2' }] + } + } + } + } + const nextProps = { + contact: { + relationships: { + related: { + data: [{ _id: '2' }, { _id: '1' }] + } + } + } + } + expect(isSameContactProp(prevProps, nextProps)).toBe(true) + }) + + it('should return false if contacts have different "related" relationships', () => { + const prevProps = { + contact: { + relationships: { + related: { + data: [{ _id: '1' }, { _id: '2' }] + } + } + } + } + const nextProps = { + contact: { + relationships: { + related: { + data: [{ _id: '1' }, { _id: '3' }] + } + } + } + } + expect(isSameContactProp(prevProps, nextProps)).toBe(false) + }) + + it('should return false if one of the contacts has no "related" relationships', () => { + const prevProps = { + contact: { + relationships: { + related: { + data: [{ _id: '1' }, { _id: '2' }] + } + } + } + } + const nextProps = { + contact: {} + } + expect(isSameContactProp(prevProps, nextProps)).toBe(false) + }) + + it('should return false if both contacts have no "related" relationships', () => { + const prevProps = { + contact: {} + } + const nextProps = { + contact: {} + } + expect(isSameContactProp(prevProps, nextProps)).toBe(false) + }) + }) })