Skip to content

Commit

Permalink
fix(edit-page-2): #27899 layout editor not reflecting template changes (
Browse files Browse the repository at this point in the history
#28066)

* Fixed reflect changes in template

* Fixed test

* Fixed test on dotcms-ui
  • Loading branch information
KevinDavilaDotCMS committed Mar 26, 2024
1 parent 99efe52 commit 4278dcc
Show file tree
Hide file tree
Showing 14 changed files with 235 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<ng-template #enabledComponent>
<dotcms-template-builder-lib
[layout]="pageState.layout"
[themeId]="pageState.template.theme"
[template]="pageState.template"
[containerMap]="containerMap"
(templateChange)="nextUpdateTemplate($event)"
data-testId="new-template-builder">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import {
DotPropertiesService,
DotRouterService,
DotSessionStorageService,
DotGlobalMessageService
DotGlobalMessageService,
DotPageStateService
} from '@dotcms/data-access';
import { DotCMSResponse, HttpCode, ResponseView } from '@dotcms/dotcms-js';
import { DotLayout, DotPageRender, DotTemplateDesigner } from '@dotcms/dotcms-models';
Expand Down Expand Up @@ -104,6 +105,12 @@ describe('DotEditLayoutComponent', () => {
DotSessionStorageService,
DotEditLayoutService,
DotRouterService,
{
provide: DotPageStateService,
useValue: {
state$: of(PAGE_STATE)
}
},
{
provide: DotHttpErrorManagerService,
useValue: {
Expand Down Expand Up @@ -357,13 +364,6 @@ describe('DotEditLayoutComponent', () => {
expect(component).toBeTruthy();
});

it('should set the themeId @Input correctly', () => {
const templateBuilder = fixture.debugElement.query(
By.css('[data-testId="new-template-builder"]')
);
expect(templateBuilder.componentInstance.themeId).toBe(PAGE_STATE.template.theme);
});

it('should emit events from new-template-builder when the layout is changed', () => {
const builder = fixture.debugElement.query(
By.css('[data-testId="new-template-builder"]')
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Subject } from 'rxjs';

import { HttpErrorResponse } from '@angular/common/http';
import { Component, HostBinding, OnDestroy, OnInit } from '@angular/core';
import { Component, HostBinding, OnDestroy, OnInit, inject, signal } from '@angular/core';
import { ActivatedRoute, Router } from '@angular/router';

import { debounceTime, filter, finalize, pluck, switchMap, take, takeUntil } from 'rxjs/operators';
Expand All @@ -13,7 +13,8 @@ import {
DotPageLayoutService,
DotRouterService,
DotSessionStorageService,
DotGlobalMessageService
DotGlobalMessageService,
DotPageStateService
} from '@dotcms/data-access';
import { ResponseView } from '@dotcms/dotcms-js';
import {
Expand Down Expand Up @@ -45,6 +46,9 @@ export class DotEditLayoutComponent implements OnInit, OnDestroy {
@HostBinding('style.minWidth') width = '100%';

private lastLayout: DotTemplateDesigner;
private pageStateStore = inject(DotPageStateService);

templateIdentifier = signal('');

constructor(
private route: ActivatedRoute,
Expand All @@ -66,14 +70,18 @@ export class DotEditLayoutComponent implements OnInit, OnDestroy {
take(1)
)
.subscribe((state: DotPageRenderState) => {
this.pageState = state;
this.updatePageState(state);

this.containerMap = this.pageState.containerMap; // containerMap from pageState is a get property, which causes to trigger a function everytime the Angular change detection runs.

const mappedContainers = this.getRemappedContainers(state.containers);
this.templateContainersCacheService.set(mappedContainers);
});

this.pageStateStore.state$.pipe(takeUntil(this.destroy$)).subscribe((state) => {
this.updatePageState(state);
});

this.saveTemplateDebounce();
this.apiLink = `api/v1/page/render${this.pageState.page.pageURI}?language_id=${this.pageState.page.languageId}`;
this.subscribeOnChangeBeforeLeaveHandler();
Expand All @@ -88,6 +96,17 @@ export class DotEditLayoutComponent implements OnInit, OnDestroy {
this.destroy$.complete();
}

/**
* Updates the page state and the template identifier with a new state.
*
* @param {DotPageRenderState} newState
* @memberof DotEditLayoutComponent
*/
updatePageState(newState: DotPageRenderState) {
this.pageState = newState;
this.templateIdentifier.set(newState.template.identifier);
}

/**
* Handle cancel in layout event
*
Expand Down Expand Up @@ -184,7 +203,7 @@ export class DotEditLayoutComponent implements OnInit, OnDestroy {
this.dotGlobalMessageService.success(
this.dotMessageService.get('dot.common.message.saved')
);
this.pageState = updatedPage;
this.templateIdentifier.set(updatedPage.template.identifier);
this.containerMap = updatedPage.containerMap; // containerMap from pageState is a get property, which causes to trigger a function everytime the Angular change detection runs.
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ export class DotEmaShellComponent implements OnInit, OnDestroy {

this.activatedRoute.data.pipe(take(1)).subscribe(({ data }) => {
this.store.load({
clientHost: data.url,
clientHost: data?.url,
...this.queryParams
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,52 @@ describe('EditEmaStore', () => {
done();
});
});

it('should return templateIdentifier', (done) => {
spectator.service.templateIdentifier$.subscribe((state) => {
expect(state).toEqual('111');
done();
});
});

it('should return layoutProperties', (done) => {
const containersMapMock = {
'/default/': {
type: 'containers',
identifier: '5363c6c6-5ba0-4946-b7af-cf875188ac2e',
name: 'Medium Column (md-1)',
categoryId: '9ab97328-e72f-4d7e-8be6-232f53218a93',
source: 'DB',
parentPermissionable: {
hostname: 'demo.dotcms.com'
}
},
'/banner/': {
type: 'containers',
identifier: '56bd55ea-b04b-480d-9e37-5d6f9217dcc3',
name: 'Large Column (lg-1)',
categoryId: 'dde0b865-6cea-4ff0-8582-85e5974cf94f',
source: 'FILE',
path: '/container/path',
parentPermissionable: {
hostname: 'demo.dotcms.com'
}
}
};
spectator.service.layoutProperties$.subscribe((state) => {
expect(state).toEqual({
layout: mockDotLayout(),
themeId: mockDotTemplate().theme,
pageId: '123',
containersMap: containersMapMock,
template: {
identifier: '111',
themeId: undefined
}
});
done();
});
});
});

describe('updaters', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ export class EditEmaStore extends ComponentStore<EditEmaState> {

readonly stateLoad$ = this.select((state) => state.editorState);

readonly templateThemeId$ = this.select((state) => state.editor.template.themeId);

readonly templateIdentifier$ = this.select((state) => state.editor.template.identifier);

readonly contentState$ = this.select(this.code$, this.stateLoad$, (code, state) => {
return {
state,
Expand Down Expand Up @@ -129,13 +133,28 @@ export class EditEmaStore extends ComponentStore<EditEmaState> {

readonly clientHost$ = this.select((state) => state.clientHost);

readonly layoutProperties$ = this.select((state) => ({
/**
* Before this was layoutProperties, but are separate to "temp" selector.
* And then is merged with templateIdentifier in layoutProperties$.
* This is to try avoid extra-calls on the select, and avoid memory leaks
*/
private readonly layoutProps$ = this.select((state) => ({
layout: state.editor.layout,
themeId: state.editor.template.theme,
pageId: state.editor.page.identifier,
containersMap: this.mapContainers(state.editor.containers)
}));

readonly layoutProperties$ = this.select(
this.layoutProps$,
this.templateIdentifier$,
this.templateThemeId$,
(props, templateIdentifier, themeId) => ({
...props,
template: { identifier: templateIdentifier, themeId }
})
);

readonly shellProperties$ = this.select((state) => ({
page: state.editor.page,
siteId: state.editor.site.identifier,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<dotcms-template-builder-lib
*ngIf="layoutProperties$ | async as layoutProperties"
[layout]="layoutProperties.layout"
[themeId]="layoutProperties.themeId"
[template]="layoutProperties.template"
[containerMap]="layoutProperties.containersMap"
(templateChange)="nextTemplateUpdate($event)"
data-testId="edit-ema-layout" />
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export interface DotTemplateBuilderState {
resizingRowID: string;
themeId: string;
shouldEmit: boolean;
templateIdentifier: string;
}

export type WidgetType = 'col' | 'row';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ describe('DotTemplateBuilderStore', () => {
}
});

service.updateOldRows(updatedRows);
service.updateOldRows({ newRows: updatedRows, templateIdentifier: '111' });

rows$.subscribe(({ rows, shouldEmit }) => {
expect(rows).toEqual(resultAfterMerge);
Expand All @@ -618,6 +618,74 @@ describe('DotTemplateBuilderStore', () => {
});
});

it('should replace the rows with the new data - when is diffrent template identifier', (done) => {
// Here i just swapped the rows and changed the uuid as expected from the backend
const swappedRows = [
{
...ROWS_MINIMAL_MOCK[1],
y: 0 // This sets the order of the rows
},
{
...ROWS_MINIMAL_MOCK[0],
y: 1 // This sets the order of the rows
}
];

// Update the containers uuid simulating the backend
const updatedRows: DotGridStackWidget[] = [
{
...ROWS_MINIMAL_MOCK[1],
id: 'random test 2',
y: 0, // This sets the order of the rows
subGridOpts: {
...ROWS_MINIMAL_MOCK[1].subGridOpts,
children: ROWS_MINIMAL_MOCK[1].subGridOpts.children.map((col) => ({
...col,
id: 'hello there 2',
containers: col.containers.map((child, i) => ({
...child,
uuid: `${i + 1}` // 1 for the 0 index
}))
}))
}
},
{
...ROWS_MINIMAL_MOCK[0],
id: 'random test 1',
y: 1, // This sets the order of the rows
subGridOpts: {
...ROWS_MINIMAL_MOCK[0].subGridOpts,
children: ROWS_MINIMAL_MOCK[0].subGridOpts.children.map((col) => ({
...col,
id: 'hello there 1',
containers: col.containers.map((child, i) => ({
...child,
uuid: `${i + 3}` // 1 for the 0 index and 2 for the first 2 containers
}))
}))
}
}
];

service.setState({
...INITIAL_STATE_MOCK,
rows: swappedRows,
layoutProperties: {
footer: false,
header: false,
sidebar: {}
}
});

service.updateOldRows({ newRows: updatedRows, templateIdentifier: '222' }); //Different template identifier

rows$.subscribe(({ rows, shouldEmit }) => {
expect(rows).toEqual(updatedRows);
expect(shouldEmit).toEqual(false);
done();
});
});

describe('Util Methods', () => {
describe('subGridOnDropped', () => {
it('should execute moveColumnInYAxis when oldNode and newNode exist', (done) => {
Expand Down

0 comments on commit 4278dcc

Please sign in to comment.