4571: Fix UI lockout when using In/Out mode with existing split columns (#4715)

* Fix catch 22 problem when using In/Out with split columns already enabled
- Also changed UI and transaction amount parsing behavior to accept all options combined
- Repositioned some amount options
- Adjusted the labels of a few options

* Fix amount options triggering state resets

* Update VRT

* Revert "Update VRT"

This reverts commit 0a4b70afad.

* Update VRT

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Matt Fiddaman <github@m.fiddaman.uk>

packages/desktop-client/src/components/modals/ImportTransactionsModal/FieldMappings.tsx

@@ -95,7 +95,7 @@ export function FieldMappings({
             firstTransaction={transactions[0]}
           />
         </View>
-        {splitMode ? (
+        {splitMode && !inOutMode ? (
           <>
             <View style={{ flex: 0.5 }}>
               <SubLabel title="Outflow" />

packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.jsx

@@ -207,6 +207,7 @@ export function ImportTransactionsModal({
       multiplierAmount,
     ) => {
       const previewTransactions = [];
+      const inOutModeEnabled = isOfxFile(filetype) ? false : inOutMode;
 
       for (let trans of transactions) {
         if (trans.isMatchedTransaction) {
@@ -237,7 +238,7 @@ export function ImportTransactionsModal({
         const { amount } = parseAmountFields(
           trans,
           splitMode,
-          isOfxFile(filetype) ? false : inOutMode,
+          inOutModeEnabled,
           outValue,
           flipAmount,
           multiplierAmount,
@@ -412,7 +413,9 @@ export function ImportTransactionsModal({
         setTransactions(transactionPreview);
       }
     },
-    [accountId, getImportPreview, inOutMode, multiplierAmount, outValue, prefs],
+    // We use some state variables from the component, but do not want to re-parse when they change
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    [accountId, getImportPreview, prefs],
   );
 
   function onMultiplierChange(e) {
@@ -449,14 +452,8 @@ export function ImportTransactionsModal({
       return;
     }
 
-    if (flipAmount === true) {
-      setFlipAmount(!flipAmount);
-    }
-
     const isSplit = !splitMode;
     setSplitMode(isSplit);
-    setInOutMode(false);
-    setFlipAmount(false);
 
     // Run auto-detection on the fields to try to detect the fields
     // automatically
@@ -1026,7 +1023,6 @@ export function ImportTransactionsModal({
                   <CheckboxOption
                     id="form_flip"
                     checked={flipAmount}
-                    disabled={splitMode || inOutMode}
                     onChange={() => {
                       setFlipAmount(!flipAmount);
                       runImportPreview();
@@ -1034,31 +1030,6 @@ export function ImportTransactionsModal({
                   >
                     {t('Flip amount')}
                   </CheckboxOption>
-                  {filetype === 'csv' && (
-                    <>
-                      <CheckboxOption
-                        id="form_split"
-                        checked={splitMode}
-                        disabled={inOutMode || flipAmount}
-                        onChange={() => {
-                          onSplitMode();
-                          runImportPreview();
-                        }}
-                      >
-                        {t('Split amount into separate inflow/outflow columns')}
-                      </CheckboxOption>
-                      <InOutOption
-                        inOutMode={inOutMode}
-                        outValue={outValue}
-                        disabled={splitMode || flipAmount}
-                        onToggle={() => {
-                          setInOutMode(!inOutMode);
-                          runImportPreview();
-                        }}
-                        onChangeText={setOutValue}
-                      />
-                    </>
-                  )}
                   <MultiplierOption
                     multiplierEnabled={multiplierEnabled}
                     multiplierAmount={multiplierAmount}
@@ -1069,6 +1040,29 @@ export function ImportTransactionsModal({
                     }}
                     onChangeAmount={onMultiplierChange}
                   />
+                  {filetype === 'csv' && (
+                    <>
+                      <CheckboxOption
+                        id="form_split"
+                        checked={splitMode}
+                        onChange={() => {
+                          onSplitMode();
+                          runImportPreview();
+                        }}
+                      >
+                        {t('Split amount into separate inflow/outflow columns')}
+                      </CheckboxOption>
+                      <InOutOption
+                        inOutMode={inOutMode}
+                        outValue={outValue}
+                        onToggle={() => {
+                          setInOutMode(!inOutMode);
+                          runImportPreview();
+                        }}
+                        onChangeText={setOutValue}
+                      />
+                    </>
+                  )}
                 </View>
               </Stack>
             </View>

packages/desktop-client/src/components/modals/ImportTransactionsModal/InOutOption.tsx

@@ -29,7 +29,7 @@ export function InOutOption({
         onChange={onToggle}
       >
         {inOutMode
-          ? 'in/out identifier'
+          ? 'In/Out outflow value'
           : 'Select column to specify if amount goes in/out'}
       </CheckboxOption>
       {inOutMode && (
@@ -37,7 +37,7 @@ export function InOutOption({
           type="text"
           value={outValue}
           onChangeValue={onChangeText}
-          placeholder="Value for out rows, i.e. Credit"
+          placeholder="Value for out rows, e.g: ‘Credit’"
         />
       )}
     </View>

packages/desktop-client/src/components/modals/ImportTransactionsModal/MultiplierOption.tsx

@@ -25,7 +25,7 @@ export function MultiplierOption({
         checked={multiplierEnabled}
         onChange={onToggle}
       >
-        Add multiplier
+        Multiply amount
       </CheckboxOption>
       <Input
         type="text"

packages/desktop-client/src/components/modals/ImportTransactionsModal/SelectField.tsx

@@ -19,20 +19,22 @@ export function SelectField({
   hasHeaderRow,
   firstTransaction,
 }: SelectFieldProps) {
+  const columns = options.map(
+    option =>
+      [
+        option,
+        hasHeaderRow
+          ? option
+          : `Column ${parseInt(option) + 1} (${firstTransaction[option]})`,
+      ] as const,
+  );
+
+  // If selected column does not exist in transaction sheet, ignore
+  if (!columns.find(col => col[0] === value)) value = null;
+
   return (
     <Select
-      options={[
+      options={[['choose-field', 'Choose field...'], ...columns]}
-        ['choose-field', 'Choose field...'],
-        ...options.map(
-          option =>
-            [
-              option,
-              hasHeaderRow
-                ? option
-                : `Column ${parseInt(option) + 1} (${firstTransaction[option]})`,
-            ] as const,
-        ),
-      ]}
       value={value === null ? 'choose-field' : value}
       onChange={onChange}
       style={style}

packages/desktop-client/src/components/modals/ImportTransactionsModal/Transaction.tsx

@@ -30,7 +30,7 @@ type TransactionProps = {
   dateFormat: ComponentProps<typeof ParsedDate>['dateFormat'];
   splitMode: boolean;
   inOutMode: boolean;
-  outValue: number;
+  outValue: string;
   flipAmount: boolean;
   multiplierAmount: string;
   categories: CategoryEntity[];
@@ -208,6 +208,19 @@ export function Transaction({
       >
         {categoryList.includes(transaction.category) && transaction.category}
       </Field>
+      {inOutMode && (
+        <Field
+          width={90}
+          contentStyle={{ textAlign: 'left', ...styles.tnum }}
+          title={
+            transaction.inOut === undefined
+              ? undefined
+              : String(transaction.inOut)
+          }
+        >
+          {transaction.inOut}
+        </Field>
+      )}
       {splitMode ? (
         <>
           <Field
@@ -246,36 +259,21 @@ export function Transaction({
           </Field>
         </>
       ) : (
-        <>
+        <Field
-          {inOutMode && (
+          width={90}
-            <Field
+          contentStyle={{
-              width={90}
+            textAlign: 'right',
-              contentStyle={{ textAlign: 'left', ...styles.tnum }}
+            ...styles.tnum,
-              title={
+            ...(amount === null ? { color: theme.errorText } : {}),
-                transaction.inOut === undefined
+          }}
-                  ? undefined
+          title={
-                  : String(transaction.inOut)
+            amount === null
-              }
+              ? `Invalid: unable to parse the value (${transaction.amount})`
-            >
+              : amountToCurrency(amount)
-              {transaction.inOut}
+          }
-            </Field>
+        >
-          )}
+          {amountToCurrency(amount || 0)}
-          <Field
+        </Field>
-            width={90}
-            contentStyle={{
-              textAlign: 'right',
-              ...styles.tnum,
-              ...(amount === null ? { color: theme.errorText } : {}),
-            }}
-            title={
-              amount === null
-                ? `Invalid: unable to parse the value (${transaction.amount})`
-                : amountToCurrency(amount)
-            }
-          >
-            {amountToCurrency(amount || 0)}
-          </Field>
-        </>
       )}
     </Row>
   );

packages/desktop-client/src/components/modals/ImportTransactionsModal/utils.ts

@@ -126,7 +126,7 @@ export type ImportTransaction = {
   amount: number;
   inflow: number;
   outflow: number;
-  inOut: number;
+  inOut: string;
 } & Record<string, string>;
 
 export type FieldMapping = {
@@ -161,7 +161,6 @@ export function applyFieldMappings(
 function parseAmount(
   amount: number | string | undefined | null,
   mapper: (parsed: number) => number,
-  multiplier: number,
 ) {
   if (amount == null) {
     return null;
@@ -174,54 +173,82 @@ function parseAmount(
     return null;
   }
 
-  return mapper(parsed) * multiplier;
+  return mapper(parsed);
 }
 
 export function parseAmountFields(
   trans: Partial<ImportTransaction>,
   splitMode: boolean,
   inOutMode: boolean,
-  outValue: number,
+  outValue: string,
   flipAmount: boolean,
   multiplierAmount: string,
 ) {
   const multiplier = parseFloat(multiplierAmount) || 1.0;
 
-  if (splitMode) {
+  /** Keep track of the transaction amount as inflow and outflow.
+   *
+   * Inflow/outflow is taken from a positive/negative transaction amount
+   * respectively, or the inflow/outflow fields if split mode is enabled.
+   */
+  const value = {
+    outflow: 0,
+    inflow: 0,
+  };
+
+  // Determine the base value of the transaction from the amount or inflow/outflow fields
+  if (splitMode && !inOutMode) {
     // Split mode is a little weird; first we look for an outflow and
     // if that has a value, we never want to show a number in the
     // inflow. Same for `amount`; we choose outflow first and then inflow
-    const outflow = parseAmount(trans.outflow, n => -Math.abs(n), multiplier);
+    value.outflow = parseAmount(trans.outflow, n => -Math.abs(n)) || 0;
-    const inflow = outflow
+    value.inflow = value.outflow
       ? 0
-      : parseAmount(trans.inflow, n => Math.abs(n), multiplier);
+      : parseAmount(trans.inflow, n => Math.abs(n)) || 0;
-
+  } else {
-    return {
+    const amount = parseAmount(trans.amount, n => n) || 0;
-      amount: outflow || inflow,
+    if (amount >= 0) value.inflow = amount;
-      outflow,
+    else value.outflow = amount;
-      inflow,
-    };
   }
+
+  // Apply in/out
   if (inOutMode) {
+    // The 'In/Out' field of a transaction will tell us
+    // whether the transaction value is inflow or outflow.
+    const transactionValue = value.outflow || value.inflow;
+    if (trans.inOut === outValue) {
+      value.outflow = -Math.abs(transactionValue);
+      value.inflow = 0;
+    } else {
+      value.inflow = Math.abs(transactionValue);
+      value.outflow = 0;
+    }
+  }
+
+  // Apply flip
+  if (flipAmount) {
+    const oldInflow = value.inflow;
+    value.inflow = Math.abs(value.outflow);
+    value.outflow = -Math.abs(oldInflow);
+  }
+
+  // Apply multiplier
+  value.inflow *= multiplier;
+  value.outflow *= multiplier;
+
+  if (splitMode) {
     return {
-      amount: parseAmount(
+      amount: value.outflow || value.inflow,
-        trans.amount,
+      outflow: value.outflow,
-        n => (trans.inOut === outValue ? Math.abs(n) * -1 : Math.abs(n)),
+      inflow: value.inflow,
-        multiplier,
+    };
-      ),
+  } else {
+    return {
+      amount: value.outflow || value.inflow,
       outflow: null,
       inflow: null,
     };
   }
-  return {
-    amount: parseAmount(
-      trans.amount,
-      n => (flipAmount ? n * -1 : n),
-      multiplier,
-    ),
-    outflow: null,
-    inflow: null,
-  };
 }
 
 export function stripCsvImportTransaction(transaction: ImportTransaction) {

upcoming-release-notes/4715.md

@@ -0,0 +1,6 @@
+---
+category: Bugfix
+authors: [Gmanicus]
+---
+
+Reworked import to accept all amount options at the same time to fix UI lockout when using In/Out mode with existing split columns.