Skip to content

Commit f8de42d

Browse files
nicobyteswezellclaude
authored
Refactor DotEditFieldDialogComponent (#36341)
This pull request refactors the field editing experience in the Content Type Fields Drop Zone component, moving from an inline dialog defined in the template to a dedicated dialog component (`DotEditFieldDialogComponent`) opened via the PrimeNG `DialogService`. It also cleans up unused code and dependencies, and simplifies how icons are determined for draggable field items. **Field editing dialog refactor:** * Replaces the large inline dialog and tab logic in the template (`content-type-fields-drop-zone.component.html`) with a service-based modal dialog, removing all dialog-related state and logic from the component and template. (`[[1]](diffhunk://#diff-d9b6e2b0fd6eea4ba0d04be0c7da85b202e97b20584cf7042bcaea398dff26caL27-L138)`, `[[2]](diffhunk://#diff-b1988fdbb798b95d236e7d8fe4d0225c7ae7d153a4490f8a7eb2a7cb197be6d9L46-L77)`, `[[3]](diffhunk://#diff-b1988fdbb798b95d236e7d8fe4d0225c7ae7d153a4490f8a7eb2a7cb197be6d9L105-L126)`, `[[4]](diffhunk://#diff-b1988fdbb798b95d236e7d8fe4d0225c7ae7d153a4490f8a7eb2a7cb197be6d9L169-L187)`, `[[5]](diffhunk://#diff-b1988fdbb798b95d236e7d8fe4d0225c7ae7d153a4490f8a7eb2a7cb197be6d9L199-R140)`, `[[6]](diffhunk://#diff-b1988fdbb798b95d236e7d8fe4d0225c7ae7d153a4490f8a7eb2a7cb197be6d9L234-R215)`, `[[7]](diffhunk://#diff-b1988fdbb798b95d236e7d8fe4d0225c7ae7d153a4490f8a7eb2a7cb197be6d9L276-L289)`, `[[8]](diffhunk://#diff-b1988fdbb798b95d236e7d8fe4d0225c7ae7d153a4490f8a7eb2a7cb197be6d9L310-L311)`, `[[9]](diffhunk://#diff-b1988fdbb798b95d236e7d8fe4d0225c7ae7d153a4490f8a7eb2a7cb197be6d9L326-R286)`, `[[10]](diffhunk://#diff-b1988fdbb798b95d236e7d8fe4d0225c7ae7d153a4490f8a7eb2a7cb197be6d9L351-L355)`) * Adds use of `DialogService` to open the new `DotEditFieldDialogComponent` and handle dialog results for saving, converting, or cancelling field edits. (`[[1]](diffhunk://#diff-b1988fdbb798b95d236e7d8fe4d0225c7ae7d153a4490f8a7eb2a7cb197be6d9L11-R31)`, `[[2]](diffhunk://#diff-b1988fdbb798b95d236e7d8fe4d0225c7ae7d153a4490f8a7eb2a7cb197be6d9L46-L77)`, `[[3]](diffhunk://#diff-b1988fdbb798b95d236e7d8fe4d0225c7ae7d153a4490f8a7eb2a7cb197be6d9L234-R215)`) **Code cleanup and dependency updates:** * Removes unused imports and providers, such as `FieldService` and `Renderer2`, from both the draggable item and drop zone components and their tests. (`[[1]](diffhunk://#diff-3235157a5fe0e93e4604adcd0a5a089e768879c9efb1414173819fd9330a8da2L19)`, `[[2]](diffhunk://#diff-3235157a5fe0e93e4604adcd0a5a089e768879c9efb1414173819fd9330a8da2L48-R47)`, `[[3]](diffhunk://#diff-545bf2f6f49386cb122451f7f59a9ceca8a40306d0bafbdf5232a06f5e4b0986L18-R18)`, `[[4]](diffhunk://#diff-545bf2f6f49386cb122451f7f59a9ceca8a40306d0bafbdf5232a06f5e4b0986L36)`, `[[5]](diffhunk://#diff-b1988fdbb798b95d236e7d8fe4d0225c7ae7d153a4490f8a7eb2a7cb197be6d9L11-R31)`) * Updates the icon logic in the draggable item component to use the `getFieldIcon` utility instead of the removed `FieldService`. (`[[1]](diffhunk://#diff-545bf2f6f49386cb122451f7f59a9ceca8a40306d0bafbdf5232a06f5e4b0986L18-R18)`, `[[2]](diffhunk://#diff-545bf2f6f49386cb122451f7f59a9ceca8a40306d0bafbdf5232a06f5e4b0986L86-R85)`) **Template and style improvements:** * Minor reordering of class attributes in the drop zone container for consistency. (`[core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/fields/content-type-fields-drop-zone/content-type-fields-drop-zone.component.htmlL1-R5](diffhunk://#diff-d9b6e2b0fd6eea4ba0d04be0c7da85b202e97b20584cf7042bcaea398dff26caL1-R5)`) These changes make the codebase more modular, maintainable, and easier to extend, while also improving the user experience for editing content type fields. This PR fixes: #35512 --------- Co-authored-by: Will Ezell <will@dotcms.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c1c40a4 commit f8de42d

34 files changed

Lines changed: 1465 additions & 1156 deletions

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ node_modules/
128128
/core-web/connect.lock
129129
/core-web/coverage
130130
/core-web/libpeerconnection.log
131+
/core-web/libs/agentic-tools/src/generated
131132
npm-debug.log
132133
yarn-error.log
133134
testem.log

core-web/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
.env
2+
/libs/agentic-tools/src/generated
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import { expect, Locator, Page } from '@playwright/test';
2+
import { DotCMSClazzes } from '@utils/dot-clazzes';
3+
4+
import { FieldsTypes, SiteorHostField, TextField } from '@models/newContentType.model';
5+
6+
/**
7+
* Page object for the Angular content-type builder
8+
* (`/dotAdmin/#/content-types-angular/edit/{id}`).
9+
*
10+
* Recovers the drag-and-drop "add field" logic from the legacy
11+
* `e2e/dotcms-e2e-node/.../contentTypeForm.page.ts` and adds edit + persistence
12+
* verification used to guard the `DotFieldService` (`saveFields`/`updateField`) flow.
13+
*/
14+
export class ContentTypeBuilderPage {
15+
constructor(private page: Page) {}
16+
17+
/** A placed field card inside the drop zone, located by its display name. */
18+
private placedField(name: string): Locator {
19+
return this.page.locator('dot-content-type-field-dragabble-item').filter({ hasText: name });
20+
}
21+
22+
/**
23+
* Open the builder for an existing content type and wait until the layout is ready
24+
* (drop zone rendered and the field palette available for dragging).
25+
*/
26+
async goToBuilder(contentTypeId: string) {
27+
await this.page.goto(`/dotAdmin/#/content-types-angular/edit/${contentTypeId}`);
28+
await this.page.getByTestId('fields-bag-0').first().waitFor({ state: 'visible' });
29+
await this.page.getByTestId(DotCMSClazzes.TEXT).first().waitFor({ state: 'visible' });
30+
}
31+
32+
/** Add a field through the builder UI (drag from the palette, then fill the dialog). */
33+
async addField(field: FieldsTypes) {
34+
switch (field.fieldType) {
35+
case 'text':
36+
await this.addTextField(field);
37+
break;
38+
case 'siteOrFolder':
39+
await this.addSiteOrFolderField(field);
40+
break;
41+
default:
42+
throw new Error(`addField does not support field type "${field.fieldType}" yet`);
43+
}
44+
}
45+
46+
async addTextField(field: TextField) {
47+
await this.dragFieldToZone(DotCMSClazzes.TEXT);
48+
await this.fillFieldDialogAndSave(field.title);
49+
}
50+
51+
async addSiteOrFolderField(field: SiteorHostField) {
52+
await this.dragFieldToZone(DotCMSClazzes.HOST_FOLDER);
53+
await this.fillFieldDialogAndSave(field.title);
54+
}
55+
56+
/**
57+
* Open an existing field's dialog (the whole card is clickable), rename it and save.
58+
* Exercises `DotFieldService.updateField` (`PUT .../fields/{id}`).
59+
*/
60+
async editField(currentName: string, newName: string) {
61+
await this.placedField(currentName).click();
62+
await this.fillFieldDialogAndSave(newName);
63+
}
64+
65+
async expectFieldPresent(name: string) {
66+
await expect(this.placedField(name)).toBeVisible();
67+
}
68+
69+
async expectFieldAbsent(name: string) {
70+
await expect(this.placedField(name)).toHaveCount(0);
71+
}
72+
73+
/** Drag a palette item (by field clazz) onto the first drop zone. */
74+
private async dragFieldToZone(clazz: string) {
75+
const source = this.page.getByTestId(clazz);
76+
const dropZone = this.page.getByTestId('fields-bag-0').first();
77+
await source.waitFor({ state: 'visible' });
78+
await source.dragTo(dropZone);
79+
}
80+
81+
/**
82+
* Fill the field-properties dialog name input and accept, waiting for the
83+
* fields persistence request to complete before returning.
84+
*/
85+
private async fillFieldDialogAndSave(name: string) {
86+
// Scope to the field-edit DynamicDialog so we never match an unrelated `.p-dialog`.
87+
const dialog = this.page.locator('p-dynamicdialog:has(dot-edit-field-dialog) .p-dialog');
88+
await dialog.first().waitFor({ state: 'visible' });
89+
90+
const nameInput = this.page.locator('input#name');
91+
await nameInput.waitFor({ state: 'visible' });
92+
await nameInput.fill(name);
93+
94+
const acceptButton = this.page.getByTestId('dotDialogAcceptAction');
95+
await expect(acceptButton).toBeEnabled();
96+
97+
// Set up the wait BEFORE the click that triggers persistence (saveFields / updateField).
98+
const responsePromise = this.page.waitForResponse(
99+
(response) =>
100+
/\/api\/v\d\/contenttype\/.+\/fields/.test(response.url()) &&
101+
['POST', 'PUT'].includes(response.request().method())
102+
);
103+
await acceptButton.click();
104+
// Fail fast if persistence errored — otherwise a 4xx/5xx would still resolve
105+
// the wait and the test could pass without the field being saved.
106+
const response = await responsePromise;
107+
expect(response.ok()).toBeTruthy();
108+
109+
await expect(dialog).toBeHidden();
110+
}
111+
}

core-web/apps/dotcms-ui-e2e/src/pages/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
export { ContentTypeBuilderPage } from './contentTypeBuilder.page';
12
export { LegacyEditContentFormPage } from './legacyEditContentForm.page';
23
export { ListingContentPage } from './listingContent.page';
34
export { ListingContentTypesPage } from './listingContentTypes.page';
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import { ContentTypeBuilderPage } from '@pages';
2+
import { test } from '@playwright/test';
3+
import { ContentType, createFakeContentType, deleteContentType } from '@requests/contentType';
4+
5+
/**
6+
* Content-type builder — add / edit / save fields.
7+
*
8+
* Drives the real builder UI (drag-drop add, click-to-edit) and verifies persistence by
9+
* reopening the content type. Guards the `DotFieldService` migration
10+
* (`saveFields` / `updateField`) and the "field-editor disables saving" regression.
11+
*
12+
* The content type shell is seeded via API; only the field operations go through the UI.
13+
*/
14+
test.describe('content type builder — fields', () => {
15+
let contentType: ContentType | null = null;
16+
let contentTypeId = '';
17+
18+
test.beforeEach(async ({ request }) => {
19+
const suffix = crypto.randomUUID().slice(0, 8);
20+
contentType = await createFakeContentType(request, {
21+
name: `E2EBuilderFields${suffix}`
22+
});
23+
contentTypeId = contentType.id;
24+
});
25+
26+
test.afterEach(async ({ request }) => {
27+
if (contentType) {
28+
await deleteContentType(request, contentType.id);
29+
contentType = null;
30+
}
31+
});
32+
33+
test('add text and site/folder fields persist after reopening @critical', async ({ page }) => {
34+
const suffix = Date.now();
35+
const textName = `E2E Text ${suffix}`;
36+
const siteName = `E2E Site ${suffix}`;
37+
38+
const builder = new ContentTypeBuilderPage(page);
39+
await builder.goToBuilder(contentTypeId);
40+
41+
await builder.addField({ title: textName, fieldType: 'text' });
42+
await builder.addField({ title: siteName, fieldType: 'siteOrFolder' });
43+
44+
// Reopen the builder — fields must come back from the server, not local state.
45+
await builder.goToBuilder(contentTypeId);
46+
47+
await builder.expectFieldPresent(textName);
48+
await builder.expectFieldPresent(siteName);
49+
});
50+
51+
test('edit a field name persists after reopening @critical', async ({ page }) => {
52+
const suffix = Date.now();
53+
const originalName = `E2E Original ${suffix}`;
54+
const renamedName = `E2E Renamed ${suffix}`;
55+
56+
const builder = new ContentTypeBuilderPage(page);
57+
await builder.goToBuilder(contentTypeId);
58+
59+
await builder.addField({ title: originalName, fieldType: 'text' });
60+
await builder.editField(originalName, renamedName);
61+
62+
// Reopen the builder and confirm the rename survived the round-trip.
63+
await builder.goToBuilder(contentTypeId);
64+
65+
await builder.expectFieldPresent(renamedName);
66+
await builder.expectFieldAbsent(originalName);
67+
});
68+
});
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/** Mirrors DotCMSClazzes from dotcms-models — local to E2E, no libs dependency. */
2+
export const DotCMSClazzes = {
3+
SIMPLE_CONTENT_TYPE: 'com.dotcms.contenttype.model.type.ImmutableSimpleContentType',
4+
ROW: 'com.dotcms.contenttype.model.field.ImmutableRowField',
5+
COLUMN: 'com.dotcms.contenttype.model.field.ImmutableColumnField',
6+
TAB_DIVIDER: 'com.dotcms.contenttype.model.field.ImmutableTabDividerField',
7+
LINE_DIVIDER: 'com.dotcms.contenttype.model.field.ImmutableLineDividerField',
8+
COLUMN_BREAK: 'contenttype.column.break',
9+
BINARY: 'com.dotcms.contenttype.model.field.ImmutableBinaryField',
10+
BLOCK_EDITOR: 'com.dotcms.contenttype.model.field.ImmutableStoryBlockField',
11+
CATEGORY: 'com.dotcms.contenttype.model.field.ImmutableCategoryField',
12+
CHECKBOX: 'com.dotcms.contenttype.model.field.ImmutableCheckboxField',
13+
CONSTANT: 'com.dotcms.contenttype.model.field.ImmutableConstantField',
14+
CUSTOM_FIELD: 'com.dotcms.contenttype.model.field.ImmutableCustomField',
15+
DATE: 'com.dotcms.contenttype.model.field.ImmutableDateField',
16+
DATE_AND_TIME: 'com.dotcms.contenttype.model.field.ImmutableDateTimeField',
17+
FILE: 'com.dotcms.contenttype.model.field.ImmutableFileField',
18+
HIDDEN: 'com.dotcms.contenttype.model.field.ImmutableHiddenField',
19+
IMAGE: 'com.dotcms.contenttype.model.field.ImmutableImageField',
20+
JSON: 'com.dotcms.contenttype.model.field.ImmutableJSONField',
21+
KEY_VALUE: 'com.dotcms.contenttype.model.field.ImmutableKeyValueField',
22+
MULTI_SELECT: 'com.dotcms.contenttype.model.field.ImmutableMultiSelectField',
23+
RADIO: 'com.dotcms.contenttype.model.field.ImmutableRadioField',
24+
RELATIONSHIP: 'com.dotcms.contenttype.model.field.ImmutableRelationshipField',
25+
SELECT: 'com.dotcms.contenttype.model.field.ImmutableSelectField',
26+
HOST_FOLDER: 'com.dotcms.contenttype.model.field.ImmutableHostFolderField',
27+
TAG: 'com.dotcms.contenttype.model.field.ImmutableTagField',
28+
TEXT: 'com.dotcms.contenttype.model.field.ImmutableTextField',
29+
TEXTAREA: 'com.dotcms.contenttype.model.field.ImmutableTextAreaField',
30+
TIME: 'com.dotcms.contenttype.model.field.ImmutableTimeField',
31+
WYSIWYG: 'com.dotcms.contenttype.model.field.ImmutableWysiwygField'
32+
} as const;

core-web/apps/dotcms-ui-e2e/src/utils/dot-content-types.mock.ts

Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,8 @@
11
import { faker } from '@faker-js/faker';
2+
import { DotCMSClazzes } from '@utils/dot-clazzes';
23

34
import type { ContentTypeFieldInput } from '../requests/contentType';
45

5-
/** Mirrors DotCMSClazzes from dotcms-models — local to E2E, no libs dependency. */
6-
const DotCMSClazzes = {
7-
SIMPLE_CONTENT_TYPE: 'com.dotcms.contenttype.model.type.ImmutableSimpleContentType',
8-
ROW: 'com.dotcms.contenttype.model.field.ImmutableRowField',
9-
COLUMN: 'com.dotcms.contenttype.model.field.ImmutableColumnField',
10-
TAB_DIVIDER: 'com.dotcms.contenttype.model.field.ImmutableTabDividerField',
11-
LINE_DIVIDER: 'com.dotcms.contenttype.model.field.ImmutableLineDividerField',
12-
COLUMN_BREAK: 'contenttype.column.break',
13-
BINARY: 'com.dotcms.contenttype.model.field.ImmutableBinaryField',
14-
BLOCK_EDITOR: 'com.dotcms.contenttype.model.field.ImmutableStoryBlockField',
15-
CATEGORY: 'com.dotcms.contenttype.model.field.ImmutableCategoryField',
16-
CHECKBOX: 'com.dotcms.contenttype.model.field.ImmutableCheckboxField',
17-
CONSTANT: 'com.dotcms.contenttype.model.field.ImmutableConstantField',
18-
CUSTOM_FIELD: 'com.dotcms.contenttype.model.field.ImmutableCustomField',
19-
DATE: 'com.dotcms.contenttype.model.field.ImmutableDateField',
20-
DATE_AND_TIME: 'com.dotcms.contenttype.model.field.ImmutableDateTimeField',
21-
FILE: 'com.dotcms.contenttype.model.field.ImmutableFileField',
22-
HIDDEN: 'com.dotcms.contenttype.model.field.ImmutableHiddenField',
23-
IMAGE: 'com.dotcms.contenttype.model.field.ImmutableImageField',
24-
JSON: 'com.dotcms.contenttype.model.field.ImmutableJSONField',
25-
KEY_VALUE: 'com.dotcms.contenttype.model.field.ImmutableKeyValueField',
26-
MULTI_SELECT: 'com.dotcms.contenttype.model.field.ImmutableMultiSelectField',
27-
RADIO: 'com.dotcms.contenttype.model.field.ImmutableRadioField',
28-
RELATIONSHIP: 'com.dotcms.contenttype.model.field.ImmutableRelationshipField',
29-
SELECT: 'com.dotcms.contenttype.model.field.ImmutableSelectField',
30-
HOST_FOLDER: 'com.dotcms.contenttype.model.field.ImmutableHostFolderField',
31-
TAG: 'com.dotcms.contenttype.model.field.ImmutableTagField',
32-
TEXT: 'com.dotcms.contenttype.model.field.ImmutableTextField',
33-
TEXTAREA: 'com.dotcms.contenttype.model.field.ImmutableTextAreaField',
34-
TIME: 'com.dotcms.contenttype.model.field.ImmutableTimeField',
35-
WYSIWYG: 'com.dotcms.contenttype.model.field.ImmutableWysiwygField'
36-
} as const;
37-
386
/** Backward-compatible clazz aliases used across E2E specs. */
397
export const IMMUTABLE_SIMPLE_CONTENT_TYPE = DotCMSClazzes.SIMPLE_CONTENT_TYPE;
408
export const IMMUTABLE_TEXT_FIELD = DotCMSClazzes.TEXT;

core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/fields/content-type-field-dragabble-item/content-type-field-dragabble-item.component.spec.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import { dotcmsContentTypeFieldBasicMock, MockDotMessageService } from '@dotcms/
1616
import { ContentTypesFieldDragabbleItemComponent } from './content-type-field-dragabble-item.component';
1717

1818
import { DotCopyLinkComponent } from '../../../../../../view/components/dot-copy-link/dot-copy-link.component';
19-
import { FieldService } from '../service';
2019

2120
describe('ContentTypesFieldDragabbleItemComponent', () => {
2221
let comp: ContentTypesFieldDragabbleItemComponent;
@@ -45,8 +44,7 @@ describe('ContentTypesFieldDragabbleItemComponent', () => {
4544
providers: [
4645
provideHttpClient(),
4746
provideHttpClientTesting(),
48-
{ provide: DotMessageService, useValue: messageServiceMock },
49-
FieldService
47+
{ provide: DotMessageService, useValue: messageServiceMock }
5048
]
5149
});
5250
}));

core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/fields/content-type-field-dragabble-item/content-type-field-dragabble-item.component.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { DotMessageService } from '@dotcms/data-access';
1515
import { DotCMSContentTypeField } from '@dotcms/dotcms-models';
1616
import { camelCase } from '@dotcms/utils';
1717

18-
import { FieldService } from '../service';
18+
import { getFieldIcon } from '../content-types-fields-list/content-types-fields-icon-map';
1919

2020
/**
2121
* This display field after being dropped into a Content Type Drop zone
@@ -33,7 +33,6 @@ import { FieldService } from '../service';
3333
})
3434
export class ContentTypesFieldDragabbleItemComponent implements OnInit {
3535
private dotMessageService = inject(DotMessageService);
36-
fieldService = inject(FieldService);
3736

3837
readonly $isSmall = input<boolean>(false, { alias: 'isSmall' });
3938
readonly $field = input.required<DotCMSContentTypeField>({ alias: 'field' });
@@ -83,7 +82,7 @@ export class ContentTypesFieldDragabbleItemComponent implements OnInit {
8382

8483
this.fieldAttributesString = this.fieldAttributesArray.join(', ');
8584

86-
this.icon = this.fieldService.getIcon(this.field.clazz);
85+
this.icon = getFieldIcon(this.field.clazz);
8786
}
8887

8988
/**

0 commit comments

Comments
 (0)