Skip to content

Commit

Permalink
Create a History of the changes inside a Template Layout/ Use this Hi… (
Browse files Browse the repository at this point in the history
#29752)

The problem here is that when you send a TemplateLayout after changed it
to a receiver we don't have a way to know witch was the movement on the
receiver because the UUID are already all recalculated, and the
multi_tree are not sent unless you sent the page too.

The fix was create a new history on the layout to know how the UUID of
each Container have been move, also I created a new version attribute to
count how many time the Layout have changed.

### Proposed Changes
* Change in the Handler to use the method to update the Layout when the
Template already exists on the receiver


https://github.com/dotCMS/core/pull/29752/files#diff-1e5a19fcdfdbe883752308cb7adf42231e860dff8300edf8b20c66f8b2b0fd39R168-R185

This method also recalculated the UUID.

* Create a new History field


https://github.com/dotCMS/core/pull/29752/files#diff-8a7533dd3f0219197b317cfcfc4848a3e61492c38f1bac1d5aeaec566025c626R50

* keep the history when update a TemplateLayout


https://github.com/dotCMS/core/pull/29752/files#diff-ce9702cf145c3dc3fd6472a323188b4897be3c98058272d8eba3f54d836f46ceR1433

* Create a new version field


https://github.com/dotCMS/core/pull/29752/files#diff-edc3e1981140c3dc84b3fbab35e5afb0ffc0e9f3f8b47673ed42da6939bf83fbR58

* recalculated the version when the TemplateLayout is saved


https://github.com/dotCMS/core/pull/29752/files#diff-ce9702cf145c3dc3fd6472a323188b4897be3c98058272d8eba3f54d836f46ceR1306

* Calculate the changes using the new History attribute on the receiver


https://github.com/dotCMS/core/pull/29752/files#diff-ce9702cf145c3dc3fd6472a323188b4897be3c98058272d8eba3f54d836f46ceR1319


https://github.com/dotCMS/core/pull/29752/files#diff-acae9e18f11a2e8737fad0e31d65f09f2e330c049e6320996de89ed5ef495ef4R26


### Checklist
- [ ] Tests
- [ ] Translations
- [ ] Security Implications Contemplated (add notes if applicable)

### Additional Info
** any additional useful context or info **

### Screenshots
Original             |  Updated
:-------------------------:|:-------------------------:
** original screenshot **  |  ** updated screenshot **
  • Loading branch information
freddyDOTCMS authored Aug 29, 2024
1 parent 525d17d commit 683ff1b
Show file tree
Hide file tree
Showing 9 changed files with 1,944 additions and 425 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import com.dotcms.publishing.DotPublishingException;
import com.dotcms.publishing.PublisherConfig;
import com.dotcms.rendering.velocity.services.TemplateLoader;
import com.dotcms.rendering.velocity.viewtools.DotTemplateTool;
import com.dotcms.util.xstream.XStreamHandler;
import com.dotmarketing.beans.Host;
import com.dotmarketing.beans.Identifier;
Expand All @@ -63,7 +64,10 @@
import com.dotmarketing.business.CacheLocator;
import com.dotmarketing.business.UserAPI;
import com.dotmarketing.exception.DotDataException;
import com.dotmarketing.exception.DotSecurityException;
import com.dotmarketing.portlets.templates.business.TemplateAPI;
import com.dotmarketing.portlets.templates.business.TemplateSaveParameters;
import com.dotmarketing.portlets.templates.design.bean.TemplateLayout;
import com.dotmarketing.portlets.templates.model.FileAssetTemplate;
import com.dotmarketing.portlets.templates.model.Template;
import com.dotmarketing.util.InodeUtils;
Expand Down Expand Up @@ -161,8 +165,7 @@ private void handleTemplates(Collection<File> templates) throws DotPublishingExc
Template existing = tAPI.find(template.getInode(), systemUser, false);
if(existing==null || !InodeUtils.isSet(existing.getIdentifier())) {
Identifier templateId = templateWrapper.getTemplateId();
Host localHost = APILocator.getHostAPI().find(templateId.getHostId(), systemUser, false);
tAPI.saveTemplate(template, localHost, systemUser, false);
saveTemplate(templateId, template);

PushPublishLogger.log(getClass(), PushPublishHandler.TEMPLATE, PushPublishAction.PUBLISH,
template.getIdentifier(), template.getInode(), template.getName(), config.getId());
Expand Down Expand Up @@ -207,4 +210,29 @@ private void handleTemplates(Collection<File> templates) throws DotPublishingExc
throw new DotPublishingException(errorMsg, e);
}
}

private void saveTemplate(Identifier templateId, Template template) throws DotDataException, DotSecurityException {

final User systemUser = APILocator.systemUser();
final Host localHost = APILocator.getHostAPI().find(templateId.getHostId(), systemUser, false);

final Template templateFromId
= APILocator.getTemplateAPI().findWorkingTemplate(templateId.getId(), systemUser, false);

if(templateFromId != null && InodeUtils.isSet(templateFromId.getIdentifier())) {
final TemplateLayout newTemplateLayout = DotTemplateTool.getTemplateLayout(template.getDrawedBody());

final TemplateSaveParameters templateSaveParameters = new TemplateSaveParameters
.Builder()
.setSite(localHost)
.setNewTemplate(template)
.setNewLayout(newTemplateLayout)
.setUseHistory(true)
.build();

tAPI.saveAndUpdateLayout(templateSaveParameters, systemUser, false);
} else {
tAPI.saveTemplate(template, localHost, systemUser, false);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1300,15 +1300,24 @@ public Template saveAndUpdateLayout(final TemplateSaveParameters templateSavePar
updateMultiTrees(templateSaveParameters, oldTemplateLayout.get(), user, respectFrontendRoles);
}

templateSaveParameters.getNewTemplate().setDrawedBody(reOrder(templateSaveParameters.getNewLayout()));
final TemplateLayout newTemplateLayout = reOrder(templateSaveParameters.getNewLayout());

if (!templateSaveParameters.isUseHistory()) {
newTemplateLayout.setVersion(oldTemplateLayout.map(templateLayout -> templateLayout.getVersion() + 1)
.orElse(1));
}

templateSaveParameters.getNewTemplate().setDrawedBody(newTemplateLayout);
templateSaveParameters.getNewTemplate().setDrawed(true);

return saveTemplate(templateSaveParameters.getNewTemplate(), templateSaveParameters.getSite(), user, respectFrontendRoles);
}

private void updateMultiTrees(final TemplateSaveParameters templateSaveParameters, TemplateLayout oldTemplateLayout,
User user, boolean respectFrontendRoles) throws DotDataException, DotSecurityException {
LayoutChanges changes = getChange(oldTemplateLayout, templateSaveParameters.getNewLayout());
final LayoutChanges changes = templateSaveParameters.isUseHistory() ?
getChangeFromHistory(oldTemplateLayout, templateSaveParameters.getNewLayout()) :
getChange(oldTemplateLayout, templateSaveParameters.getNewLayout());

final List<String> pageIds = UtilMethods.isSet(templateSaveParameters.getPageIds()) ?
templateSaveParameters.getPageIds() :
Expand All @@ -1322,6 +1331,52 @@ private void updateMultiTrees(final TemplateSaveParameters templateSaveParameter
multiTreeAPI.updateMultiTrees(changes, pageIds, currentVariant);
}

private LayoutChanges getChangeFromHistory(final TemplateLayout oldTemplateLayout, final TemplateLayout newLayout) {

final LayoutChanges.Builder builder = new LayoutChanges.Builder();
final List<ContainerUUID> oldContainers = getContainers(oldTemplateLayout);
final List<ContainerUUID> newContainers = getContainers(newLayout);
final int deltaVersion = newLayout.getVersion() - oldTemplateLayout.getVersion();

addMoveAndRemoveChangesWithHistory(oldContainers, newContainers, deltaVersion, builder);

return builder.build();
}

private static void addMoveAndRemoveChangesWithHistory(final List<ContainerUUID> oldContainers,
final List<ContainerUUID> newContainers,
final int layoutVersionDelta,
final LayoutChanges.Builder builder) {
oldContainers.stream().forEach(oldContainer -> {
final String uuid = oldContainer.getUUID();

final Optional<ContainerUUID> newContainerUUIDMatch = newContainers.stream()
.filter(newContainer -> oldContainer.getIdentifier().equals(newContainer.getIdentifier()))
.filter(newContainer -> isMatch(oldContainer, newContainer, layoutVersionDelta))
.findAny();

if (newContainerUUIDMatch.isPresent() && !uuid.equals(newContainerUUIDMatch.get().getUUID())) {
builder.change(oldContainer.getIdentifier(), oldContainer.getUUID(),
newContainerUUIDMatch.get().getUUID());
} else if (newContainerUUIDMatch.isEmpty()) {
builder.remove(oldContainer.getIdentifier(), uuid);
}
});
}

private static boolean isMatch(final ContainerUUID oldContainer, final ContainerUUID newContainer,
final int layoutVersionDelta) {

final List<String> oldHistoryUUIDs = oldContainer.getHistoryUUIDs();
int oldHistorySize = oldHistoryUUIDs.size();
int oldHistoryLastIndex = oldHistorySize - 1;
final String oldLastUUID = oldHistoryUUIDs.get(oldHistoryLastIndex);
final List<String> newHistoryUUIDs = newContainer.getHistoryUUIDs();

return newHistoryUUIDs.size() == (oldHistorySize + layoutVersionDelta) &&
newHistoryUUIDs.get(oldHistoryLastIndex).equals(oldLastUUID);
}


private static Optional<TemplateLayout> getTemplateLayoutFromDatabase(final String templateId, final User user)
throws DotDataException, DotSecurityException {
Expand Down Expand Up @@ -1374,8 +1429,8 @@ private TemplateLayout reOrder(final TemplateLayout layout) {
}

uuidByContainer.put(containerUUID.getIdentifier(), maxUUID);

containerUUID.setUuid(maxUUID.toString());
containerUUID.addUUIDTOHistory(maxUUID.toString());
});

return layout;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,52 @@
*
* - newLayout: This is the new layout. It is used in conjunction with oldTemplateLayout to calculate the changes between the two.
* After this, the {@link com.dotmarketing.beans.MultiTree} is updated according to the two layouts.
*
* - pageIds: These are the IDs of the pages that need to be updated. This parameter helps filter
* the {@link com.dotmarketing.beans.MultiTree} that will be updated.
*
* - site: This refers to the site associated with the template.
*
* - useHistory: This boolean attribute plays a significant role in determining how changes are calculated.
* Let's explain the two scenarios in more detail:
*
* Suppose we have the following TemplateLayout:
* <code>
* Row 1: Container A, UUID 1, history ["1"]
* Row 2: Container A, UUID 2, history ["2"]
* </code>
*
* Now, let's see how the behavior changes based on the value of useHistory.
*
* useHistory = FALSE: In this case, changes are calculated using the UUIDs in the newLayoutTemplate.
* For example, if we want to move the second row to the top, we would send a newTemplateLayout
* as follows with useHistory set to false:
*<code>
* Row 1: Container A, UUID 2
* Row 2: Container A, UUID 1
* </code>
*
* UUIDs are assigned from left to right and top to bottom. Since useHistory is false, the unordered UUIDs indicate
* the changes. The code will iterate through all containers in the layout and recognize that the instance with UUID 2
* has moved to the top.
*
* useHistory = TRUE: Now, let's consider the same example with useHistory set to true, and the newTemplateLayout
* is as follows:
* <code>
* Row 1: Container A, UUID 1, history ["2", "1"]
* Row 2: Container A, UUID 2, history ["1", "2"]
* </code>
*
* In this case, the history is used to determine the changes. The first container in the old layout has a history of ["1"].
* The code will search for a container with the same history in the new template. The match is:
*
* <code>
* Row 2: Container A, UUID 2, history ["1", "2"]
* </code>
*
* This match is based on the first position in the history list ("1"), which indicates that the container initially
* associated with UUID 1 has moved to UUID 2.
*
*/
public class TemplateSaveParameters {

Expand All @@ -29,13 +70,19 @@ public class TemplateSaveParameters {
private final List<String> pageIds;
private final TemplateLayout newLayout;
private final Host site;
private final boolean useHistory;

private TemplateSaveParameters(final Builder builder){
this.newTemplate = builder.newTemplate;
this.oldTemplateLayout = builder.oldTemplateLayout;
this.pageIds = builder.pageIds;
this.newLayout = builder.newLayout;
this.site = builder.site;
this.useHistory = builder.useHistory;
}

public boolean isUseHistory() {
return useHistory;
}

public Template getNewTemplate() {
Expand Down Expand Up @@ -64,6 +111,7 @@ public static class Builder {
private List<String> pageIds;
private TemplateLayout newLayout;
private Host site;
private boolean useHistory;

public Builder setNewTemplate(Template newTemplate) {
this.newTemplate = newTemplate;
Expand Down Expand Up @@ -93,5 +141,10 @@ public Builder setSite(Host site) {
public TemplateSaveParameters build (){
return new TemplateSaveParameters(this);
}

public Builder setUseHistory(boolean useHistory) {
this.useHistory = useHistory;
return this;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ private List<ContainerUUID> checkFileAssetContainers(final List<ContainerUUID> c
Logger.error(this, e.getMessage(), e);
}

containerUUIDS.add(new ContainerUUID(containerIdOrPath, containerUUID.getUUID()));
containerUUIDS.add(new ContainerUUID(containerIdOrPath, containerUUID.getUUID(), containerUUID.getHistoryUUIDs()));
}

return containerUUIDS;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
package com.dotmarketing.portlets.templates.design.bean;


import com.dotmarketing.util.UtilMethods;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import static com.dotcms.util.CollectionsUtils.list;

/**
* Provides the relationship between a Container and its instance ID in a Template's layout in
Expand All @@ -23,11 +30,57 @@ public class ContainerUUID implements Serializable{
private final String identifier;
private String uuid;

/**x
* History of Changes: This section lists all the UUIDs that have been assigned to this Container. For example,
* if the history is ["3", "1", "4"], it means the Container was initially assigned UUID 3,
* then changed to UUID 1 after a layout modification, and is currently assigned UUID 4.
*
* If the layout changes but the Container is not moved, the same UUID will be repeated in the history.
* For instance, consider the following layout:
*
* Row 1: Container A, UUID 1, history: ["1"]
* Row 2: Container A, UUID 2, history: ["2"]
* If a new row is added after the first one, the UUIDs and histories will be as follows:
*
* Row 1: Container A, UUID 1, history: ["1", "1"]
* Row 2: Container A, UUID 2, history: ["2"]
* Row 3: Container A, UUID 3, history: ["2", "3"]
* The ["1", "1"] indicates that the layout changed, but this Container was not moved.
*/
private List<String> historyUUIDs;

public ContainerUUID(final String containerIdOrPath,
final String containerInstanceID) {

this.identifier = containerIdOrPath;
this.uuid = containerInstanceID == null ? UUID_DEFAULT_VALUE : containerInstanceID;
this.historyUUIDs = isNew(containerInstanceID) ? new ArrayList<>() : list(this.uuid);
}

public ContainerUUID(final @JsonProperty("identifier") String containerIdOrPath,
final @JsonProperty("uuid") String containerInstanceID) {
final @JsonProperty("uuid") String containerInstanceID,
final @JsonProperty("historyUUIDs") List<String> historyUUIDs) {

this.identifier = containerIdOrPath;
this.uuid = containerInstanceID == null ? UUID_DEFAULT_VALUE : containerInstanceID;

if (isNew(containerInstanceID)) {
this.historyUUIDs = new ArrayList<>();
} else {
this.historyUUIDs = UtilMethods.isSet(historyUUIDs) ? new ArrayList<>(historyUUIDs) : list(this.uuid);
}
}

private boolean isNew(String containerInstanceID) {
return UUID_DEFAULT_VALUE.equals(containerInstanceID);
}

public void addUUIDTOHistory(final String uuid){
if (uuid.equals(UUID_DEFAULT_VALUE)) {
return;
}

historyUUIDs.add(uuid);
}

/**
Expand Down Expand Up @@ -66,4 +119,7 @@ public void setUuid(final String uuid){
this.uuid = uuid;
}

public List<String> getHistoryUUIDs() {
return Collections.unmodifiableList(this.historyUUIDs);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,10 @@

import java.io.Serializable;

import java.util.Collection;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import com.dotmarketing.portlets.containers.business.FileAssetContainerUtil;
import com.dotmarketing.portlets.containers.model.Container;
Expand Down Expand Up @@ -53,7 +51,11 @@ public class TemplateLayout implements Serializable {
private Body body;
private Sidebar sidebar;


/**
* This count how many times the TemplateLayout have changed, but it counts just layout movement
* it means if A Container is moved, remove or added.
*/
private int version = 1;

public String getPageWidth () {
return pageWidth;
Expand Down Expand Up @@ -271,4 +273,11 @@ private boolean isTheSameUUID(final String uuid1, final String uuid2) {
: uuid1.equals(uuid2);
}

public int getVersion() {
return version;
}

public void setVersion(int version) {
this.version = version;
}
}
Loading

0 comments on commit 683ff1b

Please sign in to comment.