Made the code more resilient to stale value prop in date filter (#17038)

Fixes : #17035

There was a bad pattern which risked introduce this bug in
`turnRecordFilterIntoRecordGqlOperationFilter` after recent refactor of
date logic and date filters, which didn't create any bug during dev time
because it wasn't tested with value combinations that existed before the
refactor.

Code has been re-organized to make it resilient to any wrong value
combination.

Also fixed a small bug that appeared during QA with `DATE` filter type,
which was blocking the save button from disappearing after a save.

Author: lucasbordeau

packages/twenty-front/src/modules/views/utils/__tests__/areViewFiltersEqual.test.ts

@@ -1,6 +1,6 @@
 import { type ViewFilter } from '@/views/types/ViewFilter';
-import { ViewFilterOperand } from 'twenty-shared/types';
 import { areViewFiltersEqual } from '@/views/utils/areViewFiltersEqual';
+import { ViewFilterOperand } from 'twenty-shared/types';
 
 describe('areViewFiltersEqual', () => {
   const baseFilter: ViewFilter = {
@@ -21,13 +21,6 @@ describe('areViewFiltersEqual', () => {
     expect(areViewFiltersEqual(filterA, filterB)).toBe(true);
   });
 
-  it('should return false when displayValue is different', () => {
-    const filterA = { ...baseFilter };
-    const filterB = { ...baseFilter, displayValue: 'different' };
-
-    expect(areViewFiltersEqual(filterA, filterB)).toBe(false);
-  });
-
   it('should return false when fieldMetadataId is different', () => {
     const filterA = { ...baseFilter };
     const filterB = { ...baseFilter, fieldMetadataId: 'field-2' };

packages/twenty-front/src/modules/views/utils/areViewFiltersEqual.ts

@@ -10,7 +10,6 @@ export const areViewFiltersEqual = (
     'viewFilterGroupId',
     'positionInViewFilterGroup',
     'value',
-    'displayValue',
     'operand',
     'subFieldName',
   ];

packages/twenty-shared/src/utils/filter/turnRecordFilterIntoGqlOperationFilter.ts

@@ -168,7 +168,10 @@ export const turnRecordFilterIntoRecordGqlOperationFilter = ({
           );
       }
     case 'DATE': {
-      if (recordFilter.operand === RecordFilterOperand.IS_RELATIVE) {
+      const itsARelativeDateFilter =
+        recordFilter.operand === RecordFilterOperand.IS_RELATIVE;
+
+      if (itsARelativeDateFilter) {
         const relativeDateFilterValue = resolveRelativeDateFilterStringified(
           recordFilter.value,
         );
@@ -204,62 +207,75 @@ export const turnRecordFilterIntoRecordGqlOperationFilter = ({
         };
       }
 
-      const nowAsPlainDate = Temporal.Now.plainDateISO(
-        filterValueDependencies.timeZone,
-      ).toString();
+      const operandIsTakingNowAsReference =
+        recordFilter.operand === RecordFilterOperand.IS_TODAY ||
+        recordFilter.operand === RecordFilterOperand.IS_IN_PAST ||
+        recordFilter.operand === RecordFilterOperand.IS_IN_FUTURE;
 
-      const plainDateFilter = recordFilter.value;
+      if (operandIsTakingNowAsReference) {
+        const nowAsPlainDate = Temporal.Now.plainDateISO(
+          filterValueDependencies.timeZone,
+        ).toString();
 
-      switch (recordFilter.operand) {
-        case RecordFilterOperand.IS_AFTER: {
-          return {
-            [correspondingFieldMetadataItem.name]: {
-              gte: plainDateFilter,
-            } as DateFilter,
-          };
-        }
-        case RecordFilterOperand.IS_BEFORE: {
-          return {
-            [correspondingFieldMetadataItem.name]: {
-              lt: plainDateFilter,
-            } as DateFilter,
-          };
+        switch (recordFilter.operand) {
+          case RecordFilterOperand.IS_IN_PAST:
+            return {
+              [correspondingFieldMetadataItem.name]: {
+                lt: nowAsPlainDate,
+              } as DateFilter,
+            };
+          case RecordFilterOperand.IS_IN_FUTURE:
+            return {
+              [correspondingFieldMetadataItem.name]: {
+                gte: nowAsPlainDate,
+              } as DateFilter,
+            };
+          case RecordFilterOperand.IS_TODAY: {
+            return {
+              [correspondingFieldMetadataItem.name]: {
+                eq: nowAsPlainDate,
+              } as DateFilter,
+            };
+          }
         }
+      } else {
+        const plainDateFilter = recordFilter.value;
 
-        case RecordFilterOperand.IS: {
-          return {
-            [correspondingFieldMetadataItem.name]: {
-              eq: plainDateFilter,
-            } as DateFilter,
-          };
-        }
-        case RecordFilterOperand.IS_IN_PAST:
-          return {
-            [correspondingFieldMetadataItem.name]: {
-              lt: nowAsPlainDate,
-            } as DateFilter,
-          };
-        case RecordFilterOperand.IS_IN_FUTURE:
-          return {
-            [correspondingFieldMetadataItem.name]: {
-              gte: nowAsPlainDate,
-            } as DateFilter,
-          };
-        case RecordFilterOperand.IS_TODAY: {
-          return {
-            [correspondingFieldMetadataItem.name]: {
-              eq: nowAsPlainDate,
-            } as DateFilter,
-          };
+        switch (recordFilter.operand) {
+          case RecordFilterOperand.IS_AFTER: {
+            return {
+              [correspondingFieldMetadataItem.name]: {
+                gte: plainDateFilter,
+              } as DateFilter,
+            };
+          }
+          case RecordFilterOperand.IS_BEFORE: {
+            return {
+              [correspondingFieldMetadataItem.name]: {
+                lt: plainDateFilter,
+              } as DateFilter,
+            };
+          }
+
+          case RecordFilterOperand.IS: {
+            return {
+              [correspondingFieldMetadataItem.name]: {
+                eq: plainDateFilter,
+              } as DateFilter,
+            };
+          }
         }
-        default:
-          throw new Error(
-            `Unknown operand ${recordFilter.operand} for ${filterType} filter`,
-          );
       }
+
+      throw new Error(
+        `Unknown operand ${recordFilter.operand} for ${filterType} filter`,
+      );
     }
     case 'DATE_TIME': {
-      if (recordFilter.operand === RecordFilterOperand.IS_RELATIVE) {
+      const itsARelativeDateTimeFilter =
+        recordFilter.operand === RecordFilterOperand.IS_RELATIVE;
+
+      if (itsARelativeDateTimeFilter) {
         const resolvedFilterValue = resolveDateTimeFilter(recordFilter);
 
         const parsedRelativeDateFilterValue =
@@ -307,81 +323,96 @@ export const turnRecordFilterIntoRecordGqlOperationFilter = ({
         };
       }
 
-      if (!isNonEmptyString(recordFilter.value)) {
-        throw new Error(`Date filter is empty`);
-      }
-
-      const resolvedDateTime = Temporal.Instant.from(recordFilter.value);
+      const operandIsTakingNowAsReference =
+        recordFilter.operand === RecordFilterOperand.IS_TODAY ||
+        recordFilter.operand === RecordFilterOperand.IS_IN_PAST ||
+        recordFilter.operand === RecordFilterOperand.IS_IN_FUTURE;
 
-      const now = Temporal.Now.zonedDateTimeISO(
-        filterValueDependencies.timeZone,
-      );
+      if (operandIsTakingNowAsReference) {
+        const now = Temporal.Now.zonedDateTimeISO(
+          filterValueDependencies.timeZone,
+        );
 
-      switch (recordFilter.operand) {
-        case RecordFilterOperand.IS_AFTER: {
-          return {
-            [correspondingFieldMetadataItem.name]: {
-              gte: resolvedDateTime.toString(),
-            } as DateTimeFilter,
-          };
+        switch (recordFilter.operand) {
+          case RecordFilterOperand.IS_IN_PAST:
+            return {
+              [correspondingFieldMetadataItem.name]: {
+                lt: now.toInstant().round('minute').toString(),
+              } as DateTimeFilter,
+            };
+          case RecordFilterOperand.IS_IN_FUTURE:
+            return {
+              [correspondingFieldMetadataItem.name]: {
+                gt: now.toInstant().round('minute').toString(),
+              } as DateTimeFilter,
+            };
+          case RecordFilterOperand.IS_TODAY: {
+            return {
+              and: [
+                {
+                  [correspondingFieldMetadataItem.name]: {
+                    gte: getPeriodStart(now, 'DAY').toInstant().toString(),
+                  } as DateTimeFilter,
+                },
+                {
+                  [correspondingFieldMetadataItem.name]: {
+                    lt: getNextPeriodStart(now, 'DAY').toInstant().toString(),
+                  } as DateTimeFilter,
+                },
+              ],
+            };
+          }
         }
-        case RecordFilterOperand.IS_BEFORE: {
-          return {
-            [correspondingFieldMetadataItem.name]: {
-              lt: resolvedDateTime.toString(),
-            } as DateTimeFilter,
-          };
+      } else {
+        if (!isNonEmptyString(recordFilter.value)) {
+          throw new Error(`Date filter is empty`);
         }
-        case RecordFilterOperand.IS: {
-          const start = resolvedDateTime
-            .toZonedDateTimeISO('UTC')
-            .with({ second: 0, millisecond: 0, microsecond: 0, nanosecond: 0 })
-            .toInstant();
 
-          const end = start.add({ minutes: 1 });
+        const resolvedDateTime = Temporal.Instant.from(recordFilter.value);
 
-          return {
-            and: [
-              {
-                [correspondingFieldMetadataItem.name]: {
-                  lt: end.toString(),
-                } as DateTimeFilter,
-              },
-              {
-                [correspondingFieldMetadataItem.name]: {
-                  gte: start.toString(),
-                } as DateTimeFilter,
-              },
-            ],
-          };
-        }
-        case RecordFilterOperand.IS_IN_PAST:
-          return {
-            [correspondingFieldMetadataItem.name]: {
-              lt: now.toInstant().round('minute').toString(),
-            } as DateTimeFilter,
-          };
-        case RecordFilterOperand.IS_IN_FUTURE:
-          return {
-            [correspondingFieldMetadataItem.name]: {
-              gt: now.toInstant().round('minute').toString(),
-            } as DateTimeFilter,
-          };
-        case RecordFilterOperand.IS_TODAY: {
-          return {
-            and: [
-              {
-                [correspondingFieldMetadataItem.name]: {
-                  gte: getPeriodStart(now, 'DAY').toInstant().toString(),
-                } as DateTimeFilter,
-              },
-              {
-                [correspondingFieldMetadataItem.name]: {
-                  lt: getNextPeriodStart(now, 'DAY').toInstant().toString(),
-                } as DateTimeFilter,
-              },
-            ],
-          };
+        switch (recordFilter.operand) {
+          case RecordFilterOperand.IS_AFTER: {
+            return {
+              [correspondingFieldMetadataItem.name]: {
+                gte: resolvedDateTime.toString(),
+              } as DateTimeFilter,
+            };
+          }
+          case RecordFilterOperand.IS_BEFORE: {
+            return {
+              [correspondingFieldMetadataItem.name]: {
+                lt: resolvedDateTime.toString(),
+              } as DateTimeFilter,
+            };
+          }
+          case RecordFilterOperand.IS: {
+            const start = resolvedDateTime
+              .toZonedDateTimeISO('UTC')
+              .with({
+                second: 0,
+                millisecond: 0,
+                microsecond: 0,
+                nanosecond: 0,
+              })
+              .toInstant();
+
+            const end = start.add({ minutes: 1 });
+
+            return {
+              and: [
+                {
+                  [correspondingFieldMetadataItem.name]: {
+                    lt: end.toString(),
+                  } as DateTimeFilter,
+                },
+                {
+                  [correspondingFieldMetadataItem.name]: {
+                    gte: start.toString(),
+                  } as DateTimeFilter,
+                },
+              ],
+            };
+          }
         }
       }