Skip to content

Commit

Permalink
fix(edit-content): issues 1-1 relationship (#31097)
Browse files Browse the repository at this point in the history
### Parent issue

#31040 

### Proposed Changes

This pull request includes changes to improve the handling of
relationship fields in the `RelationshipFieldStore` and related
utilities. The changes focus on enhancing the formatting of relationship
identifiers, improving edge case handling, and refining utility
functions for extracting relationships from contentlets.

Improvements to `RelationshipFieldStore`:

*
[`core-web/libs/edit-content/src/lib/fields/dot-edit-content-relationship-field/store/relationship-field.store.spec.ts`](diffhunk://#diff-d2a9a6233c2b1ba38c683725d2e68092978e7023e9269dbabab2fcdfa47224e2R14-R24):
Added identifiers to mock data and updated tests to validate the correct
formatting of relationship identifiers. Added new tests to handle
various edge cases, including pagination, data manipulation, and
initialization with extreme cardinality values.
[[1]](diffhunk://#diff-d2a9a6233c2b1ba38c683725d2e68092978e7023e9269dbabab2fcdfa47224e2R14-R24)
[[2]](diffhunk://#diff-d2a9a6233c2b1ba38c683725d2e68092978e7023e9269dbabab2fcdfa47224e2L177-R275)
*
[`core-web/libs/edit-content/src/lib/fields/dot-edit-content-relationship-field/store/relationship-field.store.ts`](diffhunk://#diff-a2c980da63c75c9dc2cd899475beab89cb79492b1dc0819574fc380e856e7ed1L66-R66):
Modified the `formattedRelationship` method to use identifiers instead
of IDs.

Enhancements to utility functions:

*
[`core-web/libs/edit-content/src/lib/fields/dot-edit-content-relationship-field/utils/index.spec.ts`](diffhunk://#diff-d895ed8b296eedf36d10492abea40febbb6bf54779bd4fc52d1d1c52256de1bfL8):
Removed the `mockContentlet` and replaced it with inline creation of
fake contentlets. Added tests for handling single relationships,
null/undefined variables, and non-array relationships.
[[1]](diffhunk://#diff-d895ed8b296eedf36d10492abea40febbb6bf54779bd4fc52d1d1c52256de1bfL8)
[[2]](diffhunk://#diff-d895ed8b296eedf36d10492abea40febbb6bf54779bd4fc52d1d1c52256de1bfL48-R47)
[[3]](diffhunk://#diff-d895ed8b296eedf36d10492abea40febbb6bf54779bd4fc52d1d1c52256de1bfL57-R56)
[[4]](diffhunk://#diff-d895ed8b296eedf36d10492abea40febbb6bf54779bd4fc52d1d1c52256de1bfR67-R109)
[[5]](diffhunk://#diff-d895ed8b296eedf36d10492abea40febbb6bf54779bd4fc52d1d1c52256de1bfL83-R122)
*
[`core-web/libs/edit-content/src/lib/fields/dot-edit-content-relationship-field/utils/index.ts`](diffhunk://#diff-02d1a23439f098900847a8ea3ad106b1eb8d5afe92e08186d7c9b0c82a13e5eaL23-R32):
Updated the `getRelationshipFromContentlet` function to handle various
edge cases and added detailed documentation for the function parameters
and return values.
[[1]](diffhunk://#diff-02d1a23439f098900847a8ea3ad106b1eb8d5afe92e08186d7c9b0c82a13e5eaL23-R32)
[[2]](diffhunk://#diff-02d1a23439f098900847a8ea3ad106b1eb8d5afe92e08186d7c9b0c82a13e5eaL35-R52)

### Checklist
- [x] Tests
- [x] Translations
- [x] Security Implications Contemplated (add notes if applicable)
  • Loading branch information
nicobytes authored Jan 9, 2025
1 parent 75bf2c2 commit 0db9310
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@ describe('RelationshipFieldStore', () => {
const mockData = [
createFakeContentlet({
inode: 'inode1',
identifier: 'identifier1',
id: '1'
}),
createFakeContentlet({
inode: 'inode2',
identifier: 'identifier2',
id: '2'
}),
createFakeContentlet({
inode: 'inode3',
identifier: 'identifier3',
id: '3'
})
];
Expand Down Expand Up @@ -174,12 +177,102 @@ describe('RelationshipFieldStore', () => {
describe('formattedRelationship', () => {
it('should format relationship IDs correctly', () => {
store.setData(mockData);
expect(store.formattedRelationship()).toBe('1,2,3');
expect(store.formattedRelationship()).toBe('identifier1,identifier2,identifier3');
});

it('should handle empty data', () => {
expect(store.formattedRelationship()).toBe('');
});

it('should handle single item', () => {
store.setData([mockData[0]]);
expect(store.formattedRelationship()).toBe('identifier1');
});

it('should handle data with different identifiers', () => {
const customData = [
createFakeContentlet({ identifier: 'abc123', id: 'abc123' }),
createFakeContentlet({ identifier: 'def456', id: 'def456' })
];
store.setData(customData);
expect(store.formattedRelationship()).toBe('abc123,def456');
});

it('should handle data with special characters in identifiers', () => {
const specialData = [
createFakeContentlet({ identifier: 'test-123', id: 'test-123' }),
createFakeContentlet({ identifier: 'test_456', id: 'test_456' })
];
store.setData(specialData);
expect(store.formattedRelationship()).toBe('test-123,test_456');
});
});
});

describe('Edge Cases', () => {
describe('pagination handling', () => {
it('should handle multiple page transitions correctly', () => {
// Move forward two pages
store.nextPage();
store.nextPage();
expect(store.pagination()).toEqual({
offset: 12,
currentPage: 3,
rowsPerPage: 6
});

// Move back one page
store.previousPage();
expect(store.pagination()).toEqual({
offset: 6,
currentPage: 2,
rowsPerPage: 6
});
});
});

describe('data manipulation', () => {
it('should handle deletion of non-existent item gracefully', () => {
store.setData(mockData);
const initialLength = store.data().length;
store.deleteItem('non-existent-inode');
expect(store.data().length).toBe(initialLength);
});

it('should handle multiple deletions correctly', () => {
store.setData(mockData);
store.deleteItem('inode1');
store.deleteItem('inode2');
expect(store.data().length).toBe(1);
expect(store.data()[0].inode).toBe('inode3');
});
});

describe('initialization edge cases', () => {
it('should handle initialization with empty contentlet', () => {
const emptyContentlet = createFakeContentlet({
id: 'empty',
inode: 'empty',
variable: 'relationship_field'
});
store.initialize({
cardinality: 0,
contentlet: emptyContentlet,
variable: 'relationship_field'
});
expect(store.data()).toBeDefined();
expect(store.data().length).toBe(0);
});

it('should handle extreme cardinality values', () => {
expect(() => {
store.initialize({
cardinality: 999,
contentlet: mockContentlet,
variable: 'relationship_field'
});
}).toThrowError();
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export const RelationshipFieldStore = signalStore(
formattedRelationship: computed(() => {
const data = state.data();

return data.map((item) => item.id).join(',');
return data.map((item) => item.identifier).join(',');
})
})),
withMethods((store) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import { getSelectionModeByCardinality, getRelationshipFromContentlet } from './
import { RELATIONSHIP_OPTIONS } from '../dot-edit-content-relationship-field.constants';
import { RelationshipTypes } from '../models/relationship.models';

const mockContentlet = createFakeContentlet();

describe('Relationship Field Utils', () => {
describe('getSelectionModeByCardinality', () => {
it('should return "single" for ONE_TO_ONE relationship', () => {
Expand Down Expand Up @@ -45,7 +43,7 @@ describe('Relationship Field Utils', () => {

it('should return empty array when variable is empty', () => {
const result = getRelationshipFromContentlet({
contentlet: mockContentlet,
contentlet: createFakeContentlet(),
variable: ''
});
expect(result).toEqual([]);
Expand All @@ -54,7 +52,7 @@ describe('Relationship Field Utils', () => {
it('should return array of relationships when contentlet has array relationship', () => {
const relationships = [createFakeContentlet(), createFakeContentlet()];
const contentlet = {
...mockContentlet,
...createFakeContentlet(),
testVar: relationships
};

Expand All @@ -65,9 +63,49 @@ describe('Relationship Field Utils', () => {
expect(result).toEqual(relationships);
});

it('should convert single relationship to array', () => {
const singleRelationship = createFakeContentlet();
const contentlet = {
...createFakeContentlet(),
testVar: singleRelationship
};

const result = getRelationshipFromContentlet({
contentlet,
variable: 'testVar'
});
expect(result).toEqual([singleRelationship]);
});

it('should return empty array when relationship variable is null', () => {
const contentlet = {
...createFakeContentlet(),
testVar: null
};

const result = getRelationshipFromContentlet({
contentlet,
variable: 'testVar'
});
expect(result).toEqual([]);
});

it('should return empty array when relationship variable is undefined', () => {
const contentlet = {
...createFakeContentlet(),
testVar: undefined
};

const result = getRelationshipFromContentlet({
contentlet,
variable: 'testVar'
});
expect(result).toEqual([]);
});

it('should return empty array when relationship is not an array', () => {
const contentlet = {
...mockContentlet,
...createFakeContentlet(),
testVar: 'not an array'
};

Expand All @@ -80,7 +118,7 @@ describe('Relationship Field Utils', () => {

it('should return empty array when relationship variable does not exist', () => {
const contentlet = {
...mockContentlet,
...createFakeContentlet(),
otherVar: []
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,16 @@ export function getSelectionModeByCardinality(cardinality: number) {
}

/**
* Get the relationship from the contentlet.
* Extracts relationship data from a contentlet based on the provided variable name.
*
* @param contentlet - The contentlet.
* @returns The relationship.
* @param params - The parameters object
* @param params.contentlet - The DotCMS contentlet object containing the relationship data
* @param params.variable - The variable name that identifies the relationship field in the contentlet
* @returns An array of related DotCMS contentlets. Returns empty array if:
* - The contentlet is null/undefined
* - The variable name is empty
* - The relationship field (contentlet[variable]) is null/undefined
* - The relationship field is not an array or single contentlet
*/
export function getRelationshipFromContentlet({
contentlet,
Expand All @@ -32,11 +38,16 @@ export function getRelationshipFromContentlet({
contentlet: DotCMSContentlet;
variable: string;
}): DotCMSContentlet[] {
if (!contentlet || !variable) {
if (!contentlet || !variable || !contentlet[variable]) {
return [];
}

const relationship = Array.isArray(contentlet[variable]) ? contentlet[variable] : [];
const relationship = contentlet[variable];
const isArray = Array.isArray(relationship);

return relationship;
if (!isArray && typeof relationship !== 'object') {
return [];
}

return isArray ? relationship : [relationship];
}

0 comments on commit 0db9310

Please sign in to comment.