dotCMS/core#21558 Edit Template Properties Modal makes changes get lost #1865

rjvelazco · 2022-01-31T17:13:06Z

Checklist

Tests

… template

…template

CLAassistant · 2022-01-31T17:13:17Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

fmontes · 2022-01-31T17:32:09Z

...dotcms-ui/src/app/portlets/dot-edit-page/layout/dot-edit-layout/dot-edit-layout.component.ts

@@ -1,4 +1,4 @@
-import { pluck, filter, take } from 'rxjs/operators';
+import { pluck, filter, take, debounceTime, tap } from 'rxjs/operators';


Are we using this here?

No, I'll remove it.

...-templates/dot-template-create-edit/dot-template-advanced/dot-template-advanced.component.ts

fmontes · 2022-01-31T17:41:23Z

...otcms-ui/src/app/portlets/dot-templates/dot-template-create-edit/store/dot-template.store.ts

+            switchMap((template: DotTemplateItem) => {
+                return this.didTemplateChangedBeforeSave(template);
+            }),
+            switchMap((template: DotTemplateItem) => {


Why double switchMap?

fmontes · 2022-01-31T17:41:53Z

...otcms-ui/src/app/portlets/dot-templates/dot-template-create-edit/store/dot-template.store.ts

+        return result;
+    }
+
+    private didTemplateChangedBeforeSave(template: DotTemplateItem): Observable<DotTemplateItem> {


Shouldn't this return a boolean?

fmontes · 2022-01-31T17:42:10Z

...otcms-ui/src/app/portlets/dot-templates/dot-template-create-edit/store/dot-template.store.ts

+    /**
+     * When we save the properties, we do not want to save the body/layout.
+     * Therefore, we keep the same body/layout of the working template
+     */


Please use the Document This extension.

fmontes · 2022-01-31T17:43:44Z

...otcms-ui/src/app/portlets/dot-templates/dot-template-create-edit/store/dot-template.store.ts

+        if (template.type === 'design') {
+            result = {
+                ...template,
+                layout: (currentTemplate as DotTemplateItemDesign).layout
+            };
+        } else {
+            result = {
+                ...template,
+                body: (currentTemplate as DotTemplateItemadvanced).body
+            };
+        }
+
+        return result;


Suggested change

if (template.type === 'design') {

result = {

...template,

layout: (currentTemplate as DotTemplateItemDesign).layout

};

} else {

result = {

...template,

body: (currentTemplate as DotTemplateItemadvanced).body

};

}

return result;

if (template.type === 'design') {

return {

...template,

layout: (currentTemplate as DotTemplateItemDesign).layout

};

}

return {

...template,

body: (currentTemplate as DotTemplateItemadvanced).body

};

fmontes · 2022-01-31T17:45:15Z

...rc/app/portlets/dot-templates/dot-template-create-edit/dot-template-create-edit.component.ts

+        this.store.didTemplateChanged$
+            .pipe(takeUntil(this.destroy$))
+            .subscribe(( didTemplateChanged ) => this.didTemplateChanged = didTemplateChanged);


async pipe.

fmontes · 2022-01-31T17:45:31Z

...rc/app/portlets/dot-templates/dot-template-create-edit/dot-template-create-edit.component.ts

@@ -22,9 +21,9 @@ import { Site, SiteService } from '@dotcms/dotcms-js';
 })
 export class DotTemplateCreateEditComponent implements OnInit, OnDestroy {
    vm$ = this.store.vm$;
+    didTemplateChanged = false;


observable candidate + async pipe.

fmontes · 2022-01-31T17:45:55Z

...rc/app/portlets/dot-templates/dot-template-create-edit/dot-template-create-edit.component.ts

+    /**
+     * Save changes on working template to store
+     *
+     * @memberof DotTemplateCreateEditComponent
+     */


Use Document This.

fmontes · 2022-01-31T17:46:34Z

.../app/portlets/dot-templates/dot-template-create-edit/dot-template-create-edit.component.html

-            [item]="vm.original"
+            [didTemplateChanged]="didTemplateChanged"
+            [item]="vm.working"
+            (saveDraft)="saveDraft($event)"


Are you 100% sure that we need a second save event?

The second save event is to update the working template state with the latest changes. In fact, that event does not make a request to save the changes. I will change the name of the event to not make it confusing

alfredo-dotcms · 2022-02-02T17:25:58Z

...dotcms-ui/src/app/portlets/dot-edit-page/layout/dot-edit-layout/dot-edit-layout.component.ts

+        this.updateTemplate.next(value);
+    }
+
+    onSaveTemplate(updatedPage: DotPageRender) {


alfredo-dotcms · 2022-02-02T17:26:08Z

...dotcms-ui/src/app/portlets/dot-edit-page/layout/dot-edit-layout/dot-edit-layout.component.ts

+        this.pageState = updatedPage;
+    }
+
+    onErrorSavingTemplate(err: ResponseView) {


fmontes · 2022-02-02T17:05:13Z

...dotcms-ui/src/app/portlets/dot-edit-page/layout/dot-edit-layout/dot-edit-layout.component.ts

+        this.updateTemplate.next(value);
+    }
+
+    onSaveTemplate(updatedPage: DotPageRender) {


fmontes · 2022-02-02T17:05:20Z

...dotcms-ui/src/app/portlets/dot-edit-page/layout/dot-edit-layout/dot-edit-layout.component.ts

+        this.pageState = updatedPage;
+    }
+
+    onErrorSavingTemplate(err: ResponseView) {


...dotcms-ui/src/app/portlets/dot-edit-page/layout/dot-edit-layout/dot-edit-layout.component.ts

fmontes · 2022-02-02T17:07:29Z

...-templates/dot-template-create-edit/dot-template-advanced/dot-template-advanced.component.ts

+    }
+
+    ngOnChanges(changes: SimpleChanges){
+        if( changes.didTemplateChanged ) {


Does your vscode is not running the formatter on save?

fmontes · 2022-02-02T17:28:37Z

...p/portlets/dot-templates/dot-template-create-edit/dot-template-create-edit.component.spec.ts

+                    saveTemplate: jasmine.createSpy(),
+                    updateWorkingTemplate: jasmine.createSpy(),
+                    goToTemplateList: jasmine.createSpy(),


You are repeating this, maybe create a base object and extend?

fmontes · 2022-02-02T17:30:26Z

...rc/app/portlets/dot-templates/dot-template-create-edit/dot-template-create-edit.component.ts

+     *
+     * @memberof DotTemplateCreateEditComponent
+     */
+     updateTemplate({ layout, body, themeId }: DotTemplate): void {


Suggested change

updateTemplate({ layout, body, themeId }: DotTemplate): void {

updateWorkingTemplate({ layout, body, themeId }: DotTemplate): void {

...rc/app/portlets/dot-templates/dot-template-create-edit/dot-template-create-edit.component.ts

fmontes · 2022-02-02T17:36:29Z

...rc/app/portlets/dot-templates/dot-template-create-edit/dot-template-create-edit.component.ts

+            };
+
+            // Design templates need to be save 10 seconds after the last change.
+            this.store.saveTemplateDebounce(value);


Why don't do this inside the store? for example, in updateWorkingTemplate since we know if is a design template there and they way we let all the logic in the store.

fmontes · 2022-02-02T17:38:51Z

...ms-ui/src/app/view/components/dot-edit-layout-designer/dot-edit-layout-designer.component.ts

    @Output()
    save: EventEmitter<Event> = new EventEmitter();

+    @Output()
+    updateTemplate: EventEmitter<Event> = new EventEmitter();


fmontes · 2022-02-04T17:32:24Z

...dotcms-ui/src/app/portlets/dot-edit-page/layout/dot-edit-layout/dot-edit-layout.component.ts

@@ -50,9 +55,37 @@ export class DotEditLayoutComponent implements OnInit {
                this.templateContainersCacheService.set(mappedContainers);
            });

+        this.updateTemplate


Add doc explaining why this approach and if you can the link where you found this solution.

fmontes · 2022-02-04T17:33:05Z

...p/portlets/dot-templates/dot-template-create-edit/dot-template-create-edit.component.spec.ts

@@ -82,6 +84,10 @@ const messageServiceMock = new MockDotMessageService({
    'templates.edit': 'Edit'
 });

+interface templateStoreValueType {


Suggested change

interface templateStoreValueType {

interface TemplateStoreValueType {

Types are camelcase

fmontes · 2022-02-04T20:58:04Z

...dotcms-ui/src/app/portlets/dot-edit-page/layout/dot-edit-layout/dot-edit-layout.component.ts

+    /**
+     *  The reason why we are using a Subject [updateTemplate] here is
+     *  because we can not just simply add a debounceTime to the HTTP Request
+     *  we need to reset the time everytime the observable is called.
+     *
+     *  More Information Here:
+     *          - https://stackoverflow.com/questions/35991867/angular-2-using-observable-debounce-with-http-get
+     *          - https://blog.bitsrc.io/3-ways-to-debounce-http-requests-in-angular-c407eb165ada
+     *
+     * @memberof DotEditLayoutComponent
+     */


This is not doc, the is is a comment inside the method. If we leave this here it will show up in the generated documentation.

* update release version for dotcms-ui & dotcms-webcomponents * CI: bumps version to 22.2.0-rc.2 [skip ci] * dotCMS/core#21504 Template designer: container selector breaks while there are "Unsave Changes" * CI: bumps version to 22.2.0-rc.2 [skip ci] * dotCMS/core#21436 Enable the bring back button (#1846) * CI: bumps version to 22.2.0-rc.3 [skip ci] * dotCMS/core#21540 Add a form from the Contentlet palette in edit page is not working * CI: bumps version to 22.2.0-rc.4 [skip ci] * dotCMS/core#21122 Replace What's changed with the htmldiff js library (#1855) * replace backend library * cleanup * tests * cleanup * error handler * CI: bumps version to 22.2.0-rc.5 [skip ci] * dotCMS/core#21600 Remove "Unsaved changes" from auto saving * dotCMS/core#21240 Show global message on inline editing (#1861) * dotCMS/core #21436 Prevent the redirect of the page when we are in edit mode * CI: bumps version to 22.2.0-rc.6 [skip ci] * dotCMS/core#21599 Push Publishing Workflow Dialog is missing timezoneId * CI: bumps version to 22.2.0-rc.7 [skip ci] * dotCMS/core#21552 Avoid trigger the original form submit event (#1862) * dotCMS/core#21552 Add forms from content palette error (#1868) * initial progress * cleanup * test updates * test updates * feedback * rename functions * CI: bumps version to 22.2.0-rc.8 [skip ci] * dotCMS/core#21656 Once a fileAsset is saved. the file itself can not be replaced using Save (#1872) * CI: bumps version to 22.2.0-rc.3 [skip ci] * dotCMS/core#21558 Edit Template Properties Modal makes changes get lost (#1865) * fix: Must supply a value for form control with name: 'title' * remove dot-template store service from Dot Template Advanced Component * fix: Edit Template Properties Modal makes changes get lost on advance template * fix: Edit Template Properties Modal makes changes get lost on design template * update template design * clean up * feedback * clean up * fix dot-template store tests * fix tests on dot-template-create-edit * fix dot-template-builder tests * fix dot-template tests * fix DotEditLayoutComponent * clean up * rename saveDraft to updateTemplate * add docs * feedback * feedback * clean up * feedback * clean up * fix doc * CI: bumps version to 22.2.0-rc.9 [skip ci] * Update package * Update package-lock * Clean up * Fix conflicts * CI: bumps version to 22.3.0-next.13 [skip ci] Co-authored-by: victoralfaro-dotcms <[email protected]> Co-authored-by: Rafael Velazco <[email protected]> Co-authored-by: Humberto Morera <[email protected]> Co-authored-by: Freddy Rodriguez <[email protected]>

rjvelazco added 6 commits January 29, 2022 18:23

fix: Must supply a value for form control with name: 'title'

be791f3

remove dot-template store service from Dot Template Advanced Component

0908123

fix: Edit Template Properties Modal makes changes get lost on advance…

82d8760

… template

fix: Edit Template Properties Modal makes changes get lost on design …

4e97dbd

…template

update template design

bb466df

clean up

e8a1ba9

rjvelazco marked this pull request as draft January 31, 2022 17:21

fmontes suggested changes Jan 31, 2022

View reviewed changes

rjvelazco added 6 commits January 31, 2022 15:37

feedback

4151ca5

clean up

0d66388

fix dot-template store tests

f9e8dbb

fix tests on dot-template-create-edit

cb1f12a

fix dot-template-builder tests

9e74d1c

fix dot-template tests

1d5e4bb

rjvelazco marked this pull request as ready for review February 2, 2022 13:01

rjvelazco added 3 commits February 2, 2022 11:24

fix DotEditLayoutComponent

a920a5b

clean up

6324d71

rename saveDraft to updateTemplate

a840470

alfredo-dotcms reviewed Feb 2, 2022

View reviewed changes

fmontes suggested changes Feb 2, 2022

View reviewed changes

rjvelazco added 4 commits February 2, 2022 14:07

add docs

edf277d

feedback

827daa9

feedback

fab0229

clean up

ab61472

fmontes suggested changes Feb 4, 2022

View reviewed changes

rjvelazco added 2 commits February 4, 2022 16:28

feedback

3b3be5d

clean up

acee63a

fmontes suggested changes Feb 4, 2022

View reviewed changes

fix doc

a0c801c

fmontes approved these changes Feb 4, 2022

View reviewed changes

alfredo-dotcms approved these changes Feb 7, 2022

View reviewed changes

fmontes merged commit 01e78ad into release-22.02 Feb 7, 2022

delete-merged-branch bot deleted the issue-21558-edit-template-properties-modal-makes-changes-get-lost branch February 7, 2022 18:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dotCMS/core#21558 Edit Template Properties Modal makes changes get lost #1865

dotCMS/core#21558 Edit Template Properties Modal makes changes get lost #1865

rjvelazco commented Jan 31, 2022 •

edited

Loading

CLAassistant commented Jan 31, 2022

fmontes Jan 31, 2022

rjvelazco Jan 31, 2022

fmontes Jan 31, 2022

fmontes Jan 31, 2022

fmontes Jan 31, 2022

fmontes Jan 31, 2022

fmontes Jan 31, 2022

fmontes Jan 31, 2022

fmontes Jan 31, 2022

fmontes Jan 31, 2022

rjvelazco Feb 2, 2022

alfredo-dotcms Feb 2, 2022

alfredo-dotcms Feb 2, 2022

fmontes Feb 2, 2022

fmontes Feb 2, 2022

fmontes Feb 2, 2022

fmontes Feb 2, 2022

fmontes Feb 2, 2022

fmontes Feb 2, 2022

fmontes Feb 2, 2022

fmontes Feb 4, 2022

fmontes Feb 4, 2022

fmontes Feb 4, 2022

		@@ -1,4 +1,4 @@
		import { pluck, filter, take } from 'rxjs/operators';
		import { pluck, filter, take, debounceTime, tap } from 'rxjs/operators';

	updateTemplate({ layout, body, themeId }: DotTemplate): void {
	updateWorkingTemplate({ layout, body, themeId }: DotTemplate): void {

	interface templateStoreValueType {
	interface TemplateStoreValueType {

dotCMS/core#21558 Edit Template Properties Modal makes changes get lost #1865

dotCMS/core#21558 Edit Template Properties Modal makes changes get lost #1865

Conversation

rjvelazco commented Jan 31, 2022 • edited Loading

Checklist

CLAassistant commented Jan 31, 2022

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

rjvelazco commented Jan 31, 2022 •

edited

Loading