Added Select and Radio fields. Added basic tests on both components. … #26461

KevinDavilaDotCMS · 2023-10-18T22:09:35Z

`🤖 Generated by Copilot at 22337ee`

Summary

🆕🔧🚀

This pull request adds new components and interfaces to improve the edit content form feature. It adds dot-edit-content-select-field and dot-edit-content-radio-field components to handle select and radio fields, and EditContentFormData interface to define the data for the form. It also refactors and updates the existing components and tests to use the new interface and observables.

Sing, O Muse, of the mighty pull request
That reforged the dot-edit-content-form with skill and zest
And added new components for select and radio fields
Like the cunning Hephaestus who shapes the bronze and steel

Walkthrough

Add EditContentFormData interface to define the data for the edit content form (link)
Update edit-content-layout component to fetch and transform the data for the edit content form using DotEditContentService and map operator (link, link, link, link, link)
Update edit-content-form component to accept EditContentFormData as input and create form controls with initial values (link, link, link, link)
Update edit-content-field component to render select and radio fields using new components (link, link)
Add dot-edit-content-select-field component to render a select field using p-dropdown component and mapOptions function (link, link, link, link)
Add dot-edit-content-radio-field component to render a radio field using p-radioButton component and mapOptions function (link, link, link, link)
Update templates of edit-content-form and edit-content-layout components to use layout property of formData (link, link)
Update spec files of edit-content-form and edit-content-layout components to mock EditContentFormData and test form initialization and emission (link, link, link, link, link, link, link)
Import DotFieldRequiredDirective to edit-content-field module and use it in select and radio field templates to add required indicator to field label (link, link, link)

Additional Info

** any additional useful context or info **

Screenshots

Original	Updated
original screenshot	updated screenshot

…Changed formData structure and architecture

… first option as value in select and radio field

core-web/libs/edit-content/src/lib/enums/dot-edit-content-field.enum.ts

core-web/libs/edit-content/src/lib/feature/edit-content/edit-content.layout.component.ts

...tent/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.html

zJaaal · 2023-10-19T13:36:50Z

...tent/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.html

+    <div class="field-checkbox" *ngFor="let option of options">
+        <p-radioButton
+            [inputId]="option.value"
+            [value]="option.value"
+            [formControlName]="field.variable"></p-radioButton>
+        <label class="ml-2" [for]="option.value">{{ option.label }}</label>
+    </div>


After you do the necessary changes, remember to make use of the :host

We can bind the class .field to the host.

...t/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.spec.ts

...nt/src/lib/fields/dot-edit-content-select-field/dot-edit-content-select-field.component.html

zJaaal · 2023-10-19T13:40:08Z

core-web/libs/edit-content/src/lib/utils/functions.util.ts

+        let newValue: string | number | boolean = value;
+        if (
+            dataType === DotEditContentFieldDataType.INTEGER ||
+            dataType === DotEditContentFieldDataType.FLOAT
+        ) {
+            newValue = Number(value);
+        }
+
+        if (dataType === DotEditContentFieldDataType.BOOL) {
+            newValue = value === 'true';
+        }
+
+        return { label, value: newValue };
+    });


Shouldn't we do all this process even if the label is the same as value?

...edit-content/src/lib/components/dot-edit-content-field/dot-edit-content-field.component.html

...s/edit-content/src/lib/components/dot-edit-content-field/dot-edit-content-field.component.ts

...dit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form.component.spec.ts

oidacra · 2023-10-19T15:46:33Z

...dit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form.component.spec.ts

@@ -211,7 +221,7 @@ describe('DotFormComponent', () => {
    describe('saveContent', () => {
        it('should emit the form value through the `formSubmit` event', () => {
            const component = spectator.component;
-            component.formData = LAYOUT_MOCK;
+            component.formData = CONTENT_FORM_DATA_MOCK;


Same as above, that is the reason you need to add the line 225.

oidacra · 2023-10-19T15:52:11Z

core-web/libs/edit-content/src/lib/utils/functions.util.ts

@@ -0,0 +1,35 @@
+import { DotEditContentFieldDataType } from '../enums/dot-edit-content-field.enum';
+
+export const mapOptions = (


Do you have a test for this util to be sure it do the works?

oidacra · 2023-10-19T15:52:48Z

...tent/src/lib/fields/dot-edit-content-select-field/dot-edit-content-select-field.component.ts

+@Component({
+    selector: 'dot-edit-content-select-field',
+    standalone: true,
+    imports: [CommonModule, DropdownModule, ReactiveFormsModule, DotFieldRequiredDirective],


Add only the necessary from CommonModule

oidacra · 2023-10-19T15:53:57Z

...src/lib/fields/dot-edit-content-select-field/dot-edit-content-select-field.component.spec.ts

+
+    const createComponent = createComponentFactory({
+        component: DotEditContentSelectFieldComponent,
+        imports: [CommonModule, DropdownModule, ReactiveFormsModule, DotFieldRequiredDirective],


You dont need add imports. Is a Standalone component.

...src/lib/fields/dot-edit-content-select-field/dot-edit-content-select-field.component.spec.ts

...edit-content/src/lib/components/dot-edit-content-field/dot-edit-content-field.component.html

...dit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form.component.spec.ts

fmontes · 2023-10-19T20:58:09Z

...tent/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.html

+    <div class="field-checkbox" *ngFor="let option of options">
+        <p-radioButton
+            [inputId]="option.value"
+            [value]="option.value"
+            [formControlName]="field.variable"></p-radioButton>
+        <label class="ml-2" [for]="option.value">{{ option.label }}</label>
+    </div>


We can bind the class .field to the host.

...t/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.spec.ts

fmontes · 2023-10-20T04:26:45Z

...edit-content/src/lib/components/dot-edit-content-field/dot-edit-content-field.component.html

@@ -1,18 +1,36 @@
 <ng-container [ngSwitch]="field.fieldType">
-    <div class="field" *ngSwitchCase="'Text'">
+    <div class="field">


I think we can ged rid of this div and use the :host

fmontes · 2023-10-20T04:41:44Z

...t/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.spec.ts

+        ];
+        spectator.setInput('field', RADIO_FIELD_BOOLEAN_MOCK);
+        spectator.detectChanges();
+        expect(spectator.component.options).toEqual(expectedList);


fmontes · 2023-10-20T04:42:27Z

...t/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.spec.ts

+        expect(spectator.component.options).toEqual(expectedList);
+    });
+
+    it('should have a options array as radio with Boolean dataType', () => {


Maybe you can group data types test under a describe and other as well.

fmontes · 2023-10-20T04:44:01Z

...src/lib/fields/dot-edit-content-select-field/dot-edit-content-select-field.component.spec.ts

+        ];
+        spectator.setInput('field', SELECT_FIELD_TEXT_MOCK);
+        spectator.detectChanges();
+        expect(spectator.component.options).toEqual(expectedList);


Tests the DOM, for example here you need to test that you are passing the options and other attr to the <p-dropdown>

...nt/src/lib/fields/dot-edit-content-select-field/dot-edit-content-select-field.component.html

core-web/libs/edit-content/src/lib/utils/functions.util.ts

…tton. Added more test cases

fmontes · 2023-10-21T02:06:15Z

...edit-content/src/lib/components/dot-edit-content-field/dot-edit-content-field.component.html

    <dot-edit-content-text-field
        *ngSwitchCase="fieldTypes.TEXT"
        [field]="field"
-        [attr.data-testId]="'field-' + field.variable"></dot-edit-content-text-field>
+        [attr.data-testId]="'field-' + field.variable" />


fmontes · 2023-10-21T02:07:14Z

...dit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form.component.spec.ts

@@ -211,7 +221,7 @@ describe('DotFormComponent', () => {
    describe('saveContent', () => {
        it('should emit the form value through the `formSubmit` event', () => {
            const component = spectator.component;
-            component.formData = LAYOUT_MOCK;
+            component.formData = CONTENT_FORM_DATA_MOCK;


...dit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form.component.spec.ts

fmontes · 2023-10-21T02:07:40Z

...ibs/edit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form.component.ts

 import { DotMessagePipe } from '@dotcms/ui';

+import { EditContentFormData } from '../../interfaces/dot-edit-content-form.interface';


Missing this one.

fmontes · 2023-10-21T02:07:58Z

...ibs/edit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form.component.ts

@@ -31,7 +32,7 @@ import { DotEditContentFieldComponent } from '../dot-edit-content-field/dot-edit
    changeDetection: ChangeDetectionStrategy.OnPush
 })
 export class DotEditContentFormComponent implements OnInit {
-    @Input() formData: DotCMSContentTypeLayoutRow[] = [];
+    @Input() formData!: EditContentFormData;


...tent/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.html

fmontes · 2023-10-21T02:22:46Z

...ontent/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.ts

+        this.options = mapSelectableOptions(this.field.values || '', this.field.dataType);
+
+        if (this.formControl.value === null) {
+            this.formControl.setValue(this.options[0]?.value);


Any thoughts on this?

fmontes · 2023-10-21T04:19:32Z

...t/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.spec.ts

+    beforeEach(() => {
+        spectator = createComponent();
+    });
+    describe('DotEditContentRadioFieldComponent with Text DataType', () => {


We already have DotEditContentRadioFieldComponent in the first describe, don't need to added here because in the results looks like redundant:

fmontes · 2023-10-21T04:20:12Z

...t/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.spec.ts

+            ];
+            spectator.setInput('field', RADIO_FIELD_TEXT_MOCK);
+            spectator.detectChanges();
+            expect(spectator.component.options).toEqual(expectedList);


We should be testing the DOM like:

Suggested change

expect(spectator.component.options).toEqual(expectedList);

expect(spectator.query(Dropdown).options).toEqual(expectedList);

fmontes · 2023-10-21T04:20:49Z

...src/lib/fields/dot-edit-content-select-field/dot-edit-content-select-field.component.spec.ts

+            expect(spanElement).toBeTruthy();
+            expect(spanElement.textContent).toEqual('Option 1');


Suggested change

expect(spanElement).toBeTruthy();

expect(spanElement.textContent).toEqual('Option 1');

expect(spanElement.textContent).toEqual('Option 1');

Because only this will tests both.

fmontes · 2023-10-21T04:22:15Z

core-web/libs/edit-content/src/lib/enums/dot-edit-content-field.enum.ts

I think we should unify /interfaces and /enums in /models that's what we have been doing.

core-web/libs/edit-content/src/lib/utils/mock.ts

...src/lib/fields/dot-edit-content-select-field/dot-edit-content-select-field.component.spec.ts

...edit-content/src/lib/components/dot-edit-content-field/dot-edit-content-field.component.html

zJaaal · 2023-10-23T13:27:24Z

...s/edit-content/src/lib/components/dot-edit-content-field/dot-edit-content-field.component.ts

        DotEditContentSelectFieldComponent,
        DotEditContentRadioFieldComponent


remove from here and import and export these in the DotEditContentFieldsModule

…-form' of https://github.com/dotCMS/core into 26448-edit-content-render-radio-and-select-field-to-the-form

fmontes · 2023-10-23T21:48:19Z

core-web/libs/edit-content/src/lib/feature/edit-content/edit-content.layout.component.ts

-                      return this.dotEditContentService.getContentTypeFormData(contentType);
+                      return this.dotEditContentService
+                          .getContentTypeFormData(contentType)
+                          .pipe(map((res) => ({ values: { ...contentData }, layout: res })));


Suggested change

.pipe(map((res) => ({ values: { ...contentData }, layout: res })));

.pipe(map((res) => ({ contentlet: { ...contentData }, layout: res })));

According to our talk today.

fmontes · 2023-10-24T13:17:41Z

...tent/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.html

@@ -0,0 +1,7 @@
+<div class="field-checkbox" *ngFor="let option of options">
+    <p-radioButton
+        [inputId]="option.value"


We can't prevent the user to have the same value twice which result in this:

We can use field.variable which is unique and the index of the ngFor to create a unique id.

...t/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.spec.ts

fmontes · 2023-10-24T13:22:16Z

...t/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.spec.ts

+        expectedList.forEach((option) => {
+            expect(
+                spectator
+                    .queryAll(RadioButton)
+                    .find((radioOption) => radioOption.value === option.value)
+            ).toBeTruthy();
+        });


Suggested change

expectedList.forEach((option) => {

expect(

spectator

.queryAll(RadioButton)

.find((radioOption) => radioOption.value === option.value)

).toBeTruthy();

});

expect(spectator.queryAll(RadioButton).map((radio) => radio.value)).toEqual([

'one',

'two'

]);

Also, we need to test the inputId and the label with the for (to test the semantic relationship) and the label innerText.

The same applies for all other cases but labels, labels can be tested just once.

fmontes · 2023-10-24T13:29:33Z

...t/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.spec.ts

+    it('should render radio options', () => {
+        spectator.setInput('field', RADIO_FIELD_TEXT_MOCK);
+        spectator.detectComponentChanges();
+
+        const radioOptions = spectator.queryAll('p-radiobutton');
+        expect(radioOptions.length).toBe(2);
+    });


If we test the one above correctly, we might not need this one.

fmontes · 2023-10-24T13:30:54Z

...t/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.spec.ts

+        spectator.component.formControl.setValue('one');
+        spectator.detectComponentChanges();
+
+        expect(spectator.component.formControl.value).toEqual('one');


Since you are doing this

spectator.component.formControl.setValue('one');

It doesn't make sense do this:

expect(spectator.component.formControl.value).toEqual('one');

fmontes · 2023-10-24T13:35:18Z

...t/src/lib/fields/dot-edit-content-radio-field/dot-edit-content-radio-field.component.spec.ts

+        const inputChecked = spectator.queryAll('div.p-radiobutton-checked');
+        expect(inputChecked.length).toBe(1);


PrimeNG is a library that changes the CSS classes and HTML markup in their updates frequently.

Using a selector like div.p-radiobutton-checked for the test might breaks the tests in an update without us know it... we can check the checked property of the component instance.

Suggested change

const inputChecked = spectator.queryAll('div.p-radiobutton-checked');

expect(inputChecked.length).toBe(1);

const inputChecked = spectator.queryAll(RadioButton).map(radio => radio.checked);

expect(inputChecked.length).toBe(1);

fmontes · 2023-10-24T13:38:21Z

core-web/libs/edit-content/src/lib/utils/functions.util.ts

+    return value;
+};
+
+export const createSingleSelectableFieldOptions = (


Suggested change

export const createSingleSelectableFieldOptions = (

export const getSingleSelectableFieldOptions = (

… form

dotcms-sonarqube · 2023-10-24T19:24:40Z

SonarQube Quality Gate

0 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

0.0% Coverage
0.0% Duplication

KevinDavilaDotCMS · 2023-10-24T19:37:11Z

For now we have an issue with the radio buttons.
If there are multiple options with the same "value", all those radio buttons will be selected.
It is a PrimeNg problem and its solution is pending investigation
More information => primefaces/primeng#9723

Added Select and Radio fields. Added basic tests on both components. …

22337ee

…Changed formData structure and architecture

KevinDavilaDotCMS linked an issue Oct 18, 2023 that may be closed by this pull request

Edit Content: Render Radio and Select Field to the Form #26448

Closed

KevinDavilaDotCMS requested review from zJaaal, fmontes, oidacra and rjvelazco and removed request for zJaaal October 18, 2023 22:09

KevinDavilaDotCMS added 2 commits October 18, 2023 17:31

Added validation to set, in case dont have value or defaultValue, the…

d9258e3

… first option as value in select and radio field

Added enum to DataType

e30bb75

zJaaal requested changes Oct 19, 2023

View reviewed changes

oidacra reviewed Oct 19, 2023

View reviewed changes

...edit-content/src/lib/components/dot-edit-content-field/dot-edit-content-field.component.html Outdated Show resolved Hide resolved

oidacra reviewed Oct 19, 2023

View reviewed changes

...s/edit-content/src/lib/components/dot-edit-content-field/dot-edit-content-field.component.ts Outdated Show resolved Hide resolved

oidacra reviewed Oct 19, 2023

View reviewed changes

...dit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form.component.spec.ts Outdated Show resolved Hide resolved

oidacra reviewed Oct 19, 2023

View reviewed changes

...src/lib/fields/dot-edit-content-select-field/dot-edit-content-select-field.component.spec.ts Outdated Show resolved Hide resolved

oidacra reviewed Oct 19, 2023

View reviewed changes

...src/lib/fields/dot-edit-content-select-field/dot-edit-content-select-field.component.spec.ts Show resolved Hide resolved

Added tests. Changed mocks.

dca2270

fmontes requested changes Oct 20, 2023

View reviewed changes

KevinDavilaDotCMS added 3 commits October 20, 2023 14:35

Working on PR Suggestions. Commit before merge

4bcd10c

Merged with master

46435dd

Added merge with master. Added Typing to forms on Select and Radio bu…

99a979f

…tton. Added more test cases

KevinDavilaDotCMS requested review from fmontes and zJaaal October 20, 2023 21:27

fmontes marked this pull request as ready for review October 21, 2023 02:05

fmontes reviewed Oct 21, 2023

View reviewed changes

fmontes requested changes Oct 21, 2023

View reviewed changes

zJaaal requested changes Oct 23, 2023

View reviewed changes

fmontes and others added 4 commits October 23, 2023 12:22

Pair programming select tests

86d032d

Changed tests. Changed language of mocks. Moved folders to shared/

d60b9b9

Merge branch '26448-edit-content-render-radio-and-select-field-to-the…

3330de3

…-form' of https://github.com/dotCMS/core into 26448-edit-content-render-radio-and-select-field-to-the-form

Replaced shared folder with models content

00f0e9d

fmontes requested changes Oct 23, 2023

View reviewed changes

KevinDavilaDotCMS and others added 5 commits October 23, 2023 17:18

Changed and added test to RadioButton field

5b6e9b3

Changed name on util => function

d0f82fb

Working on DotEditContentForm tests

643fe24

Solved test on DotEditContentForm

9b011e8

Pair programming tests for dot-edit-content

d619855

fmontes requested changes Oct 24, 2023

View reviewed changes

Added more tests. Reduced html on radio field. Casting values on init…

ff4f914

… form

KevinDavilaDotCMS requested review from zJaaal, fmontes and oidacra October 24, 2023 19:43

fmontes approved these changes Oct 24, 2023

View reviewed changes

zJaaal approved these changes Oct 24, 2023

View reviewed changes

fmontes merged commit b103c86 into master Oct 24, 2023
24 checks passed

fmontes deleted the 26448-edit-content-render-radio-and-select-field-to-the-form branch October 24, 2023 20:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Select and Radio fields. Added basic tests on both components. … #26461

Added Select and Radio fields. Added basic tests on both components. … #26461

KevinDavilaDotCMS commented Oct 18, 2023 •

edited by ghost

Loading

zJaaal Oct 19, 2023

fmontes Oct 19, 2023

zJaaal Oct 19, 2023

oidacra Oct 19, 2023

fmontes Oct 21, 2023

oidacra Oct 19, 2023

oidacra Oct 19, 2023

oidacra Oct 19, 2023

fmontes Oct 19, 2023

fmontes Oct 20, 2023

fmontes Oct 20, 2023

fmontes Oct 20, 2023

fmontes Oct 20, 2023

fmontes Oct 21, 2023

fmontes Oct 21, 2023

fmontes Oct 21, 2023

fmontes Oct 21, 2023

fmontes Oct 21, 2023

fmontes Oct 21, 2023

fmontes Oct 21, 2023

fmontes Oct 21, 2023

fmontes Oct 21, 2023

zJaaal Oct 23, 2023

fmontes Oct 23, 2023

fmontes Oct 24, 2023

fmontes Oct 24, 2023

fmontes Oct 24, 2023

fmontes Oct 24, 2023

fmontes Oct 24, 2023

fmontes Oct 24, 2023

fmontes Oct 24, 2023

dotcms-sonarqube bot commented Oct 24, 2023

KevinDavilaDotCMS commented Oct 24, 2023

		@@ -0,0 +1,35 @@
		import { DotEditContentFieldDataType } from '../enums/dot-edit-content-field.enum';

		export const mapOptions = (

		import { DotMessagePipe } from '@dotcms/ui';

		import { EditContentFormData } from '../../interfaces/dot-edit-content-form.interface';

	expect(spectator.component.options).toEqual(expectedList);
	expect(spectator.query(Dropdown).options).toEqual(expectedList);

		expect(spanElement).toBeTruthy();
		expect(spanElement.textContent).toEqual('Option 1');

		DotEditContentSelectFieldComponent,
		DotEditContentRadioFieldComponent

	.pipe(map((res) => ({ values: { ...contentData }, layout: res })));
	.pipe(map((res) => ({ contentlet: { ...contentData }, layout: res })));

		const inputChecked = spectator.queryAll('div.p-radiobutton-checked');
		expect(inputChecked.length).toBe(1);

	export const createSingleSelectableFieldOptions = (
	export const getSingleSelectableFieldOptions = (

Added Select and Radio fields. Added basic tests on both components. … #26461

Added Select and Radio fields. Added basic tests on both components. … #26461

Conversation

KevinDavilaDotCMS commented Oct 18, 2023 • edited by ghost Loading

🤖 Generated by Copilot at 22337ee

Summary

Walkthrough

Additional Info

Screenshots

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotcms-sonarqube bot commented Oct 24, 2023

KevinDavilaDotCMS commented Oct 24, 2023

KevinDavilaDotCMS commented Oct 18, 2023 •

edited by ghost

Loading

`🤖 Generated by Copilot at 22337ee`