From 1242a16973b0ec0e6edc7c14efb9d3e63ebfcd1e Mon Sep 17 00:00:00 2001
From: Claudia Malzer <42971992+CalamityC@users.noreply.github.com>
Date: Wed, 13 Nov 2024 16:02:38 +0100
Subject: [PATCH] ERM-3392 Changes to the field content type are not saved, but
 a success message is displayed (#1368)

* ERM-3392 Changes to the field content type are not saved, but a success message is displayed

* change agreementContentTypes initialValues on submit

* * revert submit handler changes
* fix field name, add parse and format

* fix button

* chore: Change over to just "id" in select to avoid having to do loads of field level stuff

In general, Claudia's solution is "nicer" in that it prevents ANY incorrect information from appearing in the PUT.

However, it ends up doing a bunch of extra work inside the Select which we would then need to replicate everywhere to get the "best" solution consistently.

We could (and an argument exists should) do this in a RefdataSelect type component, but then that would require special testing etc etc, and to be used in a very specific way. The easier fix (which is already used in some other places) is simply to use the id _only_.

This makes some of the "only display refdata in use" logic a bit more complex, so we maybe ought to improve that and centralise it as well.

ERM-3392

---------

Co-authored-by: EthanFreestone <54310740+EthanFreestone@users.noreply.github.com>
Co-authored-by: Ethan Freestone <ethan.freestone@k-int.co.uk>
---
 .../ContentTypesFieldArray.js                 | 45 +++++++++++--------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/src/components/AgreementSections/ContentTypesFieldArray/ContentTypesFieldArray.js b/src/components/AgreementSections/ContentTypesFieldArray/ContentTypesFieldArray.js
index 6e1a11f89..a9df6e5f9 100644
--- a/src/components/AgreementSections/ContentTypesFieldArray/ContentTypesFieldArray.js
+++ b/src/components/AgreementSections/ContentTypesFieldArray/ContentTypesFieldArray.js
@@ -1,8 +1,11 @@
-import PropTypes from 'prop-types';
 import { useEffect, useState, useRef } from 'react';
+import PropTypes from 'prop-types';
+
+import isEqual from 'lodash/isEqual';
+
 import { FormattedMessage } from 'react-intl';
 import { Field } from 'react-final-form';
-import { getRefdataValuesByDesc, requiredValidator, usePrevious } from '@folio/stripes-erm-components';
+
 import {
   Button,
   Col,
@@ -13,18 +16,9 @@ import {
 } from '@folio/stripes/components';
 import { useKiwtFieldArray } from '@k-int/stripes-kint-components';
 
-import { useAgreementsRefdata } from '../../../hooks';
-
-// Utility function to check if two arrays of scalars contain the same items (Order does not count)
-const arraysAreEqual = (a, b) => {
-  if (a.length !== b.length) {
-    return false;
-  }
+import { getRefdataValuesByDesc, requiredValidator, usePrevious } from '@folio/stripes-erm-components';
 
-  return a.every(element => {
-    return b.includes(element);
-  });
-};
+import { useAgreementsRefdata } from '../../../hooks';
 
 const [
   AGREEMENT_CONTENT_TYPE
@@ -40,8 +34,12 @@ const ContentTypesFieldArray = ({
       AGREEMENT_CONTENT_TYPE,
     ]
   });
-  const contentTypeValues = getRefdataValuesByDesc(refdata, AGREEMENT_CONTENT_TYPE);
-  const agreementContentType = contentTypeValues.map(ct => ({ value: ct.value, label: ct.label }));
+  const contentTypeSelectOptions = getRefdataValuesByDesc(refdata, AGREEMENT_CONTENT_TYPE)
+    .map(ct => ({
+      value: ct.id, // Map to id for submittal purposes
+      label: ct.label
+    }));
+
   const { items, onAddField, onDeleteField } = useKiwtFieldArray(name);
   const [contentTypeInUse, setContentTypeInUse] = useState([]);
   const contentTypeRefs = useRef([]);
@@ -51,12 +49,13 @@ const ContentTypesFieldArray = ({
   const previousCount = usePrevious(itemsLength);
 
   useEffect(() => {
-    const newContentTypeInUse = items.map(i => i?.contentType?.value).filter(x => !!x);
-    if (!arraysAreEqual(contentTypeInUse, newContentTypeInUse)) {
+    const newContentTypeInUse = items.map(i => i?.contentType?.id).filter(x => !!x);
+    if (!isEqual(contentTypeInUse, newContentTypeInUse)) {
       setContentTypeInUse(newContentTypeInUse);
     }
   }, [items, contentTypeInUse]);
 
+
   useEffect(() => {
     // the second conditional is checking if the current field to be focused is the default content type field.
     if (contentTypeRefs.current.length > 1 || (contentTypeRefs.current.length > 0 && previousCount - itemsLength === 1)) {
@@ -73,7 +72,8 @@ const ContentTypesFieldArray = ({
 
   const renderContentTypes = () => (
     items.map((act, index) => {
-      const dataOptions = agreementContentType.filter(ct => !contentTypeInUse.includes(ct.value) || ct.value === act.contentType?.value);
+      const dataOptions = contentTypeSelectOptions.filter(ct => !contentTypeInUse.includes(ct.value) || ct.value === act.contentType?.id);
+
       return (
         <div
           key={`${act?.contentType?.id}`}
@@ -81,13 +81,20 @@ const ContentTypesFieldArray = ({
         >
           <Row>
             <Col xs={11}>
+              {/*
+                * This select will ONLY change the id of the refdata,
+                * and the bind will happen from that. That will make
+                * the PUT look a little funky as it will have the old
+                * label/value information, but prevents us from having
+                * to do loads of eaxtra tweaking at the field level
+                */}
               <Field
                 component={Select}
                 dataOptions={dataOptions}
                 id="content-type-field-array"
                 index={index}
                 inputRef={addToRefs}
-                name={`${name}[${index}].contentType.value`}
+                name={`${name}[${index}].contentType.id`}
                 placeholder=" "
                 validate={requiredValidator}
               />