Fix view filter update and deletion propagation (#12082)

# Introduction

Diff description: ~500 tests and +500 additions

close https://github.com/twentyhq/core-team-issues/issues/731

## What has been done here
In a nutshell on a field metadata type ( `SELECT MULTI_SELECT` ) update,
we will be browsing all `ViewFilters` in a post hook searching for some
referencing related updated `fieldMetadata` select. In order to update
or delete the `viewFilter` depending on the associated mutations.

## How to test:
- Add FieldMetadata `SELECT | MULTI_SELECT` to an existing or a new
`objectMetadata`
- Create a filtered view on created `fieldMetadata` with any options you
would like
- Remove some options ( in the best of the world some that are selected
by the filter ) from the `fieldMetadata` settings page
- Go back to the filtered view, removed or updated options should have
been hydrated in the `displayValue` and the filtered data should make
sense

## All filtered options are deleted edge case
If an update implies that a viewFilter does not have any existing
related options anymore, then we remove the viewFilter

## Testing
```sh 
PASS  test/integration/metadata/suites/field-metadata/update-one-field-metadata-related-record.integration-spec.ts (27 s)
  update-one-field-metadata-related-record
    SELECT
      ✓ should delete related view filter if all select field options got deleted (2799 ms)
      ✓ should update related multi selected options view filter (1244 ms)
      ✓ should update related solo selected option view filter (1235 ms)
      ✓ should handle partial deletion of selected options in view filter (1210 ms)
      ✓ should handle reordering of options while maintaining view filter values (1487 ms)
      ✓ should handle no changes update of options while maintaining existing view filter values (1174 ms)
      ✓ should handle adding new options while maintaining existing view filter (1174 ms)
      ✓ should update display value with options label if less than 3 options are selected (1249 ms)
      ✓ should throw error if view filter value is not a stringified JSON array (1300 ms)
    MULTI_SELECT
      ✓ should delete related view filter if all select field options got deleted (1127 ms)
      ✓ should update related multi selected options view filter (1215 ms)
      ✓ should update related solo selected option view filter (1404 ms)
      ✓ should handle partial deletion of selected options in view filter (1936 ms)
      ✓ should handle reordering of options while maintaining view filter values (1261 ms)
      ✓ should handle no changes update of options while maintaining existing view filter values (1831 ms)
      ✓ should handle adding new options while maintaining existing view filter (1610 ms)
      ✓ should update display value with options label if less than 3 options are selected (1889 ms)
      ✓ should throw error if view filter value is not a stringified JSON array (1365 ms)

Test Suites: 1 passed, 1 total
Tests:       18 passed, 18 total
Snapshots:   18 passed, 18 total
Time:        27.039 s
```
## Out of scope
- We should handle ViewFilter validation when extracting its definition
from the metadata
https://github.com/twentyhq/core-team-issues/issues/1009

## Concerns
- Are we able through the api to update an RATING fieldMetadata ? ( if
yes than that's an issue and we should handle RATING the same way than
for SELECT and MULTI_SELECT )
- It's not possible to group a view from a MULTI_SELECT field

The above points create a double nor a triple "lecture" to the post hook
effect:
- ViewGroup -> only SELECT
- VIewFilter -> only SELECT || MULTI_SELECT
- Rating nothing
I think we should determine the scope of all of that

---------

Co-authored-by: Charles Bochet <charles@twenty.com>
This commit is contained in:
Paul Rastoin
2025-05-28 12:22:28 +02:00
committed by GitHub
parent e63cd39a5b
commit 97d4ec96af
31 changed files with 1250 additions and 93 deletions

View File

@ -42,7 +42,7 @@ import {
} from 'src/engine/metadata-modules/field-metadata/utils/compute-column-name.util';
import { generateNullable } from 'src/engine/metadata-modules/field-metadata/utils/generate-nullable';
import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util';
import { isSelectFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-select-field-metadata-type.util';
import { isSelectOrMultiSelectFieldMetadata } from 'src/engine/metadata-modules/field-metadata/utils/is-select-or-multi-select-field-metadata.util';
import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
import { assertMutationNotOnRemoteObject } from 'src/engine/metadata-modules/object-metadata/utils/assert-mutation-not-on-remote-object.util';
import {
@ -255,12 +255,18 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
if (
updatedFieldMetadata.isActive &&
isSelectFieldMetadataType(updatedFieldMetadata.type)
isSelectOrMultiSelectFieldMetadata(updatedFieldMetadata) &&
isSelectOrMultiSelectFieldMetadata(existingFieldMetadata)
) {
await this.fieldMetadataRelatedRecordsService.updateRelatedViewGroups(
existingFieldMetadata,
updatedFieldMetadata,
);
await this.fieldMetadataRelatedRecordsService.updateRelatedViewFilters(
existingFieldMetadata,
updatedFieldMetadata,
);
}
if (

View File

@ -0,0 +1,347 @@
import { EachTestingContext } from 'twenty-shared/testing';
import { FieldMetadataDefaultOption } from 'src/engine/metadata-modules/field-metadata/dtos/options.input';
import { FieldMetadataRelatedRecordsService } from 'src/engine/metadata-modules/field-metadata/services/field-metadata-related-records.service';
import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
type GetOptionsDifferencesTestContext = EachTestingContext<{
oldOptions: FieldMetadataDefaultOption[];
newOptions: FieldMetadataDefaultOption[];
compareLabel?: boolean;
expected: {
created: FieldMetadataDefaultOption[];
updated: {
old: FieldMetadataDefaultOption;
new: FieldMetadataDefaultOption;
}[];
deleted: FieldMetadataDefaultOption[];
};
}>;
describe('FieldMetadataRelatedRecordsService', () => {
describe('getOptionsDifferences', () => {
let service: FieldMetadataRelatedRecordsService;
let twentyORMGlobalManager: TwentyORMGlobalManager;
beforeEach(() => {
twentyORMGlobalManager = {} as TwentyORMGlobalManager;
service = new FieldMetadataRelatedRecordsService(twentyORMGlobalManager);
});
const testCases: GetOptionsDifferencesTestContext[] = [
{
title: 'should identify created options',
context: {
oldOptions: [
{ id: '1', label: 'Option 1', value: 'value1', position: 0 },
],
newOptions: [
{ id: '1', label: 'Option 1', value: 'value1', position: 0 },
{ id: '2', label: 'Option 2', value: 'value2', position: 1 },
],
expected: {
created: [
{ id: '2', label: 'Option 2', value: 'value2', position: 1 },
],
updated: [],
deleted: [],
},
},
},
{
title: 'should identify updated options',
context: {
oldOptions: [
{ id: '1', label: 'Option 1', value: 'value1', position: 0 },
],
newOptions: [
{
id: '1',
label: 'Option 1',
value: 'updated-value1',
position: 0,
},
],
expected: {
created: [],
updated: [
{
old: {
id: '1',
label: 'Option 1',
value: 'value1',
position: 0,
},
new: {
id: '1',
label: 'Option 1',
value: 'updated-value1',
position: 0,
},
},
],
deleted: [],
},
},
},
{
title: 'should identify deleted options',
context: {
oldOptions: [
{ id: '1', label: 'Option 1', value: 'value1', position: 0 },
{ id: '2', label: 'Option 2', value: 'value2', position: 1 },
],
newOptions: [
{ id: '1', label: 'Option 1', value: 'value1', position: 0 },
],
expected: {
created: [],
updated: [],
deleted: [
{ id: '2', label: 'Option 2', value: 'value2', position: 1 },
],
},
},
},
{
title: 'should identify all types of changes',
context: {
oldOptions: [
{ id: '1', label: 'Option 1', value: 'value1', position: 0 },
{ id: '2', label: 'Option 2', value: 'value2', position: 1 },
{ id: '3', label: 'Option 3', value: 'value3', position: 2 },
],
newOptions: [
{
id: '1',
label: 'Option 1',
value: 'updated-value1',
position: 0,
},
{ id: '3', label: 'Option 3', value: 'value3', position: 1 },
{ id: '4', label: 'Option 4', value: 'value4', position: 2 },
],
expected: {
created: [
{ id: '4', label: 'Option 4', value: 'value4', position: 2 },
],
updated: [
{
old: {
id: '1',
label: 'Option 1',
value: 'value1',
position: 0,
},
new: {
id: '1',
label: 'Option 1',
value: 'updated-value1',
position: 0,
},
},
],
deleted: [
{ id: '2', label: 'Option 2', value: 'value2', position: 1 },
],
},
},
},
{
title: 'should handle empty arrays',
context: {
oldOptions: [],
newOptions: [],
expected: {
created: [],
updated: [],
deleted: [],
},
},
},
{
title: 'should handle all new options',
context: {
oldOptions: [],
newOptions: [
{ id: '1', label: 'Option 1', value: 'value1', position: 0 },
{ id: '2', label: 'Option 2', value: 'value2', position: 1 },
],
expected: {
created: [
{ id: '1', label: 'Option 1', value: 'value1', position: 0 },
{ id: '2', label: 'Option 2', value: 'value2', position: 1 },
],
updated: [],
deleted: [],
},
},
},
{
title: 'should handle all deleted options',
context: {
oldOptions: [
{ id: '1', label: 'Option 1', value: 'value1', position: 0 },
{ id: '2', label: 'Option 2', value: 'value2', position: 1 },
],
newOptions: [],
expected: {
created: [],
updated: [],
deleted: [
{ id: '1', label: 'Option 1', value: 'value1', position: 0 },
{ id: '2', label: 'Option 2', value: 'value2', position: 1 },
],
},
},
},
{
title:
'should not consider changes to label as updates when value remains the same',
context: {
oldOptions: [
{
id: 'f86eaffd-b773-4c9a-957b-86dca4a62731',
label: 'Option 0',
value: 'option0',
position: 1,
},
{
id: '28d80b3c-79bd-4a1b-a868-9616534de0fa',
label: 'Option 1',
value: 'option1',
position: 2,
},
{
id: '25a05cd8-256f-4652-9e4a-6d9ca0b96f4d',
label: 'Option 2',
value: 'option2',
position: 3,
},
],
newOptions: [
{
id: 'f86eaffd-b773-4c9a-957b-86dca4a62731',
label: 'Option 0_UPDATED', // Label changed but value remains the same
value: 'option0',
position: 1,
},
{
id: '28d80b3c-79bd-4a1b-a868-9616534de0fa',
label: 'Option 1', // No change
value: 'option1',
position: 2,
},
{
id: '25a05cd8-256f-4652-9e4a-6d9ca0b96f4d',
label: 'Option 2_UPDATED', // Label changed but value remains the same
value: 'option2',
position: 3,
},
],
expected: {
created: [],
updated: [], // No updates because only labels changed, not values
deleted: [],
},
},
},
{
title:
'should consider changes to label as updates when value remains the same if compareLabel is true',
context: {
oldOptions: [
{
id: 'f86eaffd-b773-4c9a-957b-86dca4a62731',
label: 'Option 0',
value: 'option0',
position: 1,
},
{
id: '28d80b3c-79bd-4a1b-a868-9616534de0fa',
label: 'Option 1',
value: 'option1',
position: 2,
},
{
id: '25a05cd8-256f-4652-9e4a-6d9ca0b96f4d',
label: 'Option 2',
value: 'option2',
position: 3,
},
],
newOptions: [
{
id: 'f86eaffd-b773-4c9a-957b-86dca4a62731',
label: 'Option 0_UPDATED', // Label changed but value remains the same
value: 'option0',
position: 1,
},
{
id: '28d80b3c-79bd-4a1b-a868-9616534de0fa',
label: 'Option 1', // No change
value: 'option1',
position: 2,
},
{
id: '25a05cd8-256f-4652-9e4a-6d9ca0b96f4d',
label: 'Option 2_UPDATED', // Label changed but value remains the same
value: 'option2',
position: 3,
},
],
expected: {
created: [],
updated: [
{
new: {
id: 'f86eaffd-b773-4c9a-957b-86dca4a62731',
label: 'Option 0_UPDATED',
position: 1,
value: 'option0',
},
old: {
id: 'f86eaffd-b773-4c9a-957b-86dca4a62731',
label: 'Option 0',
position: 1,
value: 'option0',
},
},
{
new: {
id: '25a05cd8-256f-4652-9e4a-6d9ca0b96f4d',
label: 'Option 2_UPDATED',
position: 3,
value: 'option2',
},
old: {
id: '25a05cd8-256f-4652-9e4a-6d9ca0b96f4d',
label: 'Option 2',
position: 3,
value: 'option2',
},
},
],
deleted: [],
},
compareLabel: true,
},
},
];
test.each(testCases)(
'$title',
({ context: { oldOptions, newOptions, expected, compareLabel } }) => {
const result = service.getOptionsDifferences(
oldOptions,
newOptions,
compareLabel,
);
expect(result.created).toEqual(expected.created);
expect(result.updated).toEqual(expected.updated);
expect(result.deleted).toEqual(expected.deleted);
},
);
});
});

View File

@ -1,15 +1,22 @@
import { Injectable } from '@nestjs/common';
import { MAX_OPTIONS_TO_DISPLAY } from 'twenty-shared/constants';
import { isDefined, parseJson } from 'twenty-shared/utils';
import { In } from 'typeorm';
import {
FieldMetadataComplexOption,
FieldMetadataDefaultOption,
} from 'src/engine/metadata-modules/field-metadata/dtos/options.input';
import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
import {
FieldMetadataException,
FieldMetadataExceptionCode,
} from 'src/engine/metadata-modules/field-metadata/field-metadata.exception';
import { isSelectFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-select-field-metadata-type.util';
import { SelectOrMultiSelectFieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/utils/is-select-or-multi-select-field-metadata.util';
import { WorkspaceRepository } from 'src/engine/twenty-orm/repository/workspace.repository';
import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
import { ViewFilterWorkspaceEntity } from 'src/modules/view/standard-objects/view-filter.workspace-entity';
import { ViewGroupWorkspaceEntity } from 'src/modules/view/standard-objects/view-group.workspace-entity';
import { ViewWorkspaceEntity } from 'src/modules/view/standard-objects/view.workspace-entity';
@ -19,6 +26,10 @@ type Differences<T> = {
deleted: T[];
};
type GetOptionsDifferences = Differences<
FieldMetadataDefaultOption | FieldMetadataComplexOption
>;
@Injectable()
export class FieldMetadataRelatedRecordsService {
constructor(
@ -26,17 +37,20 @@ export class FieldMetadataRelatedRecordsService {
) {}
public async updateRelatedViewGroups(
oldFieldMetadata: FieldMetadataEntity,
newFieldMetadata: FieldMetadataEntity,
oldFieldMetadata: SelectOrMultiSelectFieldMetadataEntity,
newFieldMetadata: SelectOrMultiSelectFieldMetadataEntity,
): Promise<void> {
// TODO legacy should support multi-select and rating ?
if (
!isSelectFieldMetadataType(newFieldMetadata.type) ||
!isSelectFieldMetadataType(oldFieldMetadata.type)
) {
return;
}
const views = await this.getFieldMetadataViews(newFieldMetadata);
const views = await this.getFieldMetadataViewWithRelation(
newFieldMetadata,
'viewGroups',
);
const { created, updated, deleted } = this.getOptionsDifferences(
oldFieldMetadata.options,
@ -100,8 +114,113 @@ export class FieldMetadataRelatedRecordsService {
}
}
private computeViewFilterDisplayValue(
newViewFilterOptions: FieldMetadataDefaultOption[],
): string {
if (newViewFilterOptions.length > MAX_OPTIONS_TO_DISPLAY) {
return `${newViewFilterOptions.length} options`;
}
return newViewFilterOptions.map((option) => option.label).join(', ');
}
public async updateRelatedViewFilters(
oldFieldMetadata: SelectOrMultiSelectFieldMetadataEntity,
newFieldMetadata: SelectOrMultiSelectFieldMetadataEntity,
): Promise<void> {
const views = await this.getFieldMetadataViewWithRelation(
newFieldMetadata,
'viewFilters',
);
const alsoCompareLabel = true;
const {
updated: updatedFieldMetadataOptions,
deleted: deletedFieldMetadataOptions,
} = this.getOptionsDifferences(
oldFieldMetadata.options,
newFieldMetadata.options,
alsoCompareLabel,
);
if (
updatedFieldMetadataOptions.length === 0 &&
deletedFieldMetadataOptions.length === 0
) {
return;
}
const viewFilterRepository =
await this.twentyORMGlobalManager.getRepositoryForWorkspace<ViewFilterWorkspaceEntity>(
newFieldMetadata.workspaceId,
'viewFilter',
);
for (const filter of views) {
if (filter.viewFilters.length === 0) {
continue;
}
for (const viewFilter of filter.viewFilters) {
const viewFilterValue = parseJson<string[]>(viewFilter.value);
// Note below assertion could be removed after https://github.com/twentyhq/core-team-issues/issues/1009 completion
if (!isDefined(viewFilterValue) || !Array.isArray(viewFilterValue)) {
throw new FieldMetadataException(
`Unexpected invalid view filter value for filter ${viewFilter.id}`,
FieldMetadataExceptionCode.INTERNAL_SERVER_ERROR,
);
}
const viewFilterOptions = viewFilterValue
.map((value) =>
oldFieldMetadata.options.find((option) => option.value === value),
)
.filter(isDefined);
const afterDeleteViewFilterOptions = viewFilterOptions.filter(
(viewFilterOption) =>
!deletedFieldMetadataOptions.some(
(option) => option.value === viewFilterOption.value,
),
);
if (afterDeleteViewFilterOptions.length === 0) {
await viewFilterRepository.delete({ id: viewFilter.id });
continue;
}
const afterUpdateAndDeleteViewFilterOptions =
afterDeleteViewFilterOptions.map((viewFilterOption) => {
const updatedOption = updatedFieldMetadataOptions.find(
({ old }) => viewFilterOption.value === old.value,
);
return isDefined(updatedOption)
? updatedOption.new
: viewFilterOption;
});
const displayValue = this.computeViewFilterDisplayValue(
afterUpdateAndDeleteViewFilterOptions,
);
const value = JSON.stringify(
afterUpdateAndDeleteViewFilterOptions.map((option) => option.value),
);
await viewFilterRepository.update(
{ id: viewFilter.id },
{
value,
displayValue,
},
);
}
}
}
async syncNoValueViewGroup(
fieldMetadata: FieldMetadataEntity,
fieldMetadata: SelectOrMultiSelectFieldMetadataEntity,
view: ViewWorkspaceEntity,
viewGroupRepository: WorkspaceRepository<ViewGroupWorkspaceEntity>,
): Promise<void> {
@ -125,10 +244,11 @@ export class FieldMetadataRelatedRecordsService {
}
}
private getOptionsDifferences(
public getOptionsDifferences(
oldOptions: (FieldMetadataDefaultOption | FieldMetadataComplexOption)[],
newOptions: (FieldMetadataDefaultOption | FieldMetadataComplexOption)[],
): Differences<FieldMetadataDefaultOption | FieldMetadataComplexOption> {
compareLabel = false,
): GetOptionsDifferences {
const differences: Differences<
FieldMetadataDefaultOption | FieldMetadataComplexOption
> = {
@ -138,18 +258,26 @@ export class FieldMetadataRelatedRecordsService {
};
const oldOptionsMap = new Map(oldOptions.map((opt) => [opt.id, opt]));
const newOptionsMap = new Map(newOptions.map((opt) => [opt.id, opt]));
for (const newOption of newOptions) {
const oldOption = oldOptionsMap.get(newOption.id);
if (!oldOption) {
if (!isDefined(oldOption)) {
differences.created.push(newOption);
} else if (oldOption.value !== newOption.value) {
continue;
}
if (
oldOption.value !== newOption.value ||
(compareLabel && oldOption.label !== newOption.label)
) {
differences.updated.push({ old: oldOption, new: newOption });
continue;
}
}
const newOptionsMap = new Map(newOptions.map((opt) => [opt.id, opt]));
for (const oldOption of oldOptions) {
if (!newOptionsMap.has(oldOption.id)) {
differences.deleted.push(oldOption);
@ -159,8 +287,9 @@ export class FieldMetadataRelatedRecordsService {
return differences;
}
private async getFieldMetadataViews(
fieldMetadata: FieldMetadataEntity,
private async getFieldMetadataViewWithRelation(
fieldMetadata: SelectOrMultiSelectFieldMetadataEntity,
relation: keyof Pick<ViewWorkspaceEntity, 'viewGroups' | 'viewFilters'>,
): Promise<ViewWorkspaceEntity[]> {
const viewRepository =
await this.twentyORMGlobalManager.getRepositoryForWorkspace<ViewWorkspaceEntity>(
@ -170,11 +299,11 @@ export class FieldMetadataRelatedRecordsService {
return viewRepository.find({
where: {
viewGroups: {
[relation]: {
fieldMetadataId: fieldMetadata.id,
},
},
relations: ['viewGroups'],
relations: [relation],
});
}

View File

@ -0,0 +1,18 @@
import { FieldMetadataType } from 'twenty-shared/types';
import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
export type SelectOrMultiSelectFieldMetadataEntity = FieldMetadataEntity<
FieldMetadataType.SELECT | FieldMetadataType.MULTI_SELECT
>;
export const isSelectOrMultiSelectFieldMetadata = (
fieldMetadata: unknown,
): fieldMetadata is SelectOrMultiSelectFieldMetadataEntity => {
if (!(fieldMetadata instanceof FieldMetadataEntity)) {
return false;
}
return [FieldMetadataType.SELECT, FieldMetadataType.MULTI_SELECT].includes(
fieldMetadata.type,
);
};