diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/web/SaveAction.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/web/SaveAction.java index a8cb1f23fa8..fa42f50d63f 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/web/SaveAction.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/web/SaveAction.java @@ -93,14 +93,9 @@ public class SaveAction extends EditAction private static final Logger LOGGER = LoggerFactory.getLogger(SaveAction.class); /** - * The key to retrieve the saved object version from the context. + * The key used to store the JSON answer for the save action on the XWiki context. */ - private static final String SAVED_OBJECT_VERSION_KEY = "SaveAction.savedObjectVersion"; - - /** - * The context key to know if a document has been merged for saving it. - */ - private static final String MERGED_DOCUMENTS = "SaveAction.mergedDocuments"; + static final String JSON_ANSWER_KEY = "SaveAction.jsonAnswer"; /** * Parameter value used with forceSave to specify that the merge should be performed even if there is conflicts. @@ -238,64 +233,101 @@ public boolean save(XWikiContext context) throws XWikiException tdoc.readFromForm(form, context); } + // Remove the redirect object if the save request doesn't update it. This allows users to easily overwrite + // redirect place-holders that are created when we move pages around. + if (tdoc.getXObject(RedirectClassDocumentInitializer.REFERENCE) != null + && request.getParameter("XWiki.RedirectClass_0_location") == null) { + tdoc.removeXObjects(RedirectClassDocumentInitializer.REFERENCE); + } + + // There are cases (e.g. when leaving a realtime collaboration session) when we don't want to create a new + // document revision unless there are actual changes. This is indicated by setting the preventEmptyRevision + // request parameter to true. Changing the document authors marks the document metadata as dirty, which is why + // we need to remember the dirty state before updating the authors, if we want to prevent empty revisions. Note + // that the document can be further modified by the save event listeners called below, but we expect them to set + // a version summary comment if they want to force the save even when preventEmptyRevision is true. + boolean dirtyBeforeSettingAuthors = tdoc.isContentDirty() || tdoc.isMetaDataDirty(); + + // Update the document authors. UserReference currentUserReference = this.currentUserResolver.resolve(CurrentUserReference.INSTANCE); tdoc.getAuthors().setOriginalMetadataAuthor(currentUserReference); request.getEffectiveAuthor().ifPresent(tdoc.getAuthors()::setEffectiveMetadataAuthor); - if (tdoc.isNew()) { tdoc.getAuthors().setCreator(currentUserReference); } - // Make sure we have at least the meta data dirty status - tdoc.setMetaDataDirty(true); - - // Validate the document if we have xvalidate=1 in the request - if ("1".equals(request.getParameter("xvalidate"))) { - boolean validationResult = tdoc.validate(context); - // If the validation fails we should show the "Inline form" edit mode - if (validationResult == false) { - // Set display context to 'edit' - context.put("display", "edit"); - // Set the action used by the "Inline form" edit mode as the context action. See #render(XWikiContext). - context.setAction(tdoc.getDefaultEditMode(context)); - // Set the document in the context - context.put("doc", doc); - context.put("cdoc", tdoc); - context.put("tdoc", tdoc); - // Force the "Inline form" edit mode. - getCurrentScriptContext().setAttribute("editor", "inline", ScriptContext.ENGINE_SCOPE); - - return true; - } + // Validate the document if we have xvalidate=1 in the request. + if ("1".equals(request.getParameter("xvalidate")) && !tdoc.validate(context)) { + // Validation failed. Redirect to "Inline form" edit mode. + // Set display context to "edit". + context.put("display", "edit"); + // Set the action used by the "Inline form" edit mode as the context action. See #render(XWikiContext). + context.setAction(tdoc.getDefaultEditMode(context)); + // Set the document in the context. + context.put("doc", doc); + context.put("cdoc", tdoc); + context.put("tdoc", tdoc); + // Force the "Inline form" edit mode. + getCurrentScriptContext().setAttribute("editor", "inline", ScriptContext.ENGINE_SCOPE); + return true; } - // Remove the redirect object if the save request doesn't update it. This allows users to easily overwrite - // redirect place-holders that are created when we move pages around. - if (tdoc.getXObject(RedirectClassDocumentInitializer.REFERENCE) != null - && request.getParameter("XWiki.RedirectClass_0_location") == null) { - tdoc.removeXObjects(RedirectClassDocumentInitializer.REFERENCE); - } + // Prepare the JSON answer that will be sent to the editor in case of an async request. We put it on the XWiki + // context so that other parts of the code (e.g. event listeners) can add more information if needed. + Map jsonAnswer = new LinkedHashMap<>(); + context.put(JSON_ANSWER_KEY, jsonAnswer); - // We only proceed on the check between versions in case of AJAX request, so we currently stay in the edit form - // This can be improved later by displaying a nice UI with some merge options in a sync request. - // For now we don't want our user to loose their changes. - if (isConflictCheckEnabled() && Utils.isAjaxRequest(context) - && request.getParameter("previousVersion") != null) { - if (isConflictingWithVersion(context, originalDoc, tdoc)) { - return true; - } + // Detect merge conflicts. We do this only if the previous version is known, otherwise a 3-way merge is not + // possible, and the save request is asynchronous, i.e. the user is waiting in edit mode for the save result. If + // a merge conflict is detected the editor will display the merge conflict modal. + if (isConflictCheckEnabled() && Utils.isAjaxRequest(context) && request.getParameter("previousVersion") != null + && isConflictingWithVersion(context, originalDoc, tdoc)) { + return true; } - // Make sure the user is allowed to make this modification + // Make sure the user is allowed to save the document. This triggers an event whose listeners can block (cancel) + // the save if needed. We trigger the event even if the document is not dirty (i.e. doesn't need to be saved) + // because: + // * the listeners can change the document (i.e. even if the document is not dirty now, it may become dirty + // after the event listeners are called) + // * the listeners may want to block a hierarchy template from being applied, even if the root document is not + // itself affected. xwiki.checkSavingDocument(context.getUserReference(), tdoc, tdoc.getComment(), tdoc.isMinorEdit(), context); - // We get the comment to be used from the document - // It was read using readFromForm - xwiki.saveDocument(tdoc, tdoc.getComment(), tdoc.isMinorEdit(), context); - this.temporaryAttachmentSessionsManager.removeUploadedAttachments(tdoc.getDocumentReference()); + // Note that users may save an existing document without making any changes just to update the author, e.g. to + // change access rights. In this or other similar cases the version summary comment can be used to justify the + // action, which is why we force the save if a comment is provided. We consider preventEmptyRevision to be false + // by default for backward compatibility. This is currently used by realtime collaboration to avoid creating + // empty revisions when leaving the editing session. + if (tdoc.isNew() || dirtyBeforeSettingAuthors || StringUtils.isNotEmpty(tdoc.getComment()) + || !"true".equals(request.getParameter("preventEmptyRevision"))) { + // Make sure we have at least the meta data dirty otherwise the version is not incremented. + tdoc.setMetaDataDirty(true); + + // We take the version summary comment from the document being saved because it may have been modified by an + // event listener after it was copied from the EditForm by the call to readFromForm. + xwiki.saveDocument(tdoc, tdoc.getComment(), tdoc.isMinorEdit(), context); + } else { + // Let the editor know that the document was not saved because there were no changes. + jsonAnswer.put("noChanges", "true"); + } - context.put(SAVED_OBJECT_VERSION_KEY, tdoc.getRCSVersion()); + // Return the latest version number to the editor that triggered the save. We do this even if we didn't create a + // new revision because the editor may have an outdated version number. This can happen for instance if two + // users save the same document state one after the other (either because both didn't make any changes, or + // because both made the same changes). Having an up-to-date version number is important for properly detecting + // and handling merge conflicts on save. + jsonAnswer.put("newVersion", tdoc.getRCSVersion().toString()); + + // Cleanup temporary attachments that were uploaded while editing the document. We do this even if we didn't + // create a new revision (e.g. if there were no changes) because those attachments are not needed anymore (e.g. + // the user may have discarded the changes that lead to the upload of those attachments). + this.temporaryAttachmentSessionsManager.removeUploadedAttachments(tdoc.getDocumentReference()); + // If a template hierarchy is specified we start a job to copy the child pages. Even if we didn't create a new + // revision for the root document (because there were no changes) the child pages may still need to be created + // or updated, because the template can be applied on existing documents, even on documents where the template + // was previously applied. Job createJob = startCreateJob(tdoc.getDocumentReference(), form); if (createJob != null) { if (isAsync(request)) { @@ -479,7 +511,7 @@ private boolean isConflictingWithVersion(XWikiContext context, XWikiDocument ori // then we pursue to save the document. if (FORCE_SAVE_MERGE.equals(request.getParameter("forceSave")) || !mergeDocumentResult.hasConflicts()) { - context.put(MERGED_DOCUMENTS, "true"); + getJSONAnswer(context).put("mergedDocument", "true"); return false; // If we got merge conflicts and we don't want to force it, then we record the conflict in @@ -509,6 +541,12 @@ private boolean isConflictingWithVersion(XWikiContext context, XWikiDocument ori return false; } + @SuppressWarnings("unchecked") + Map getJSONAnswer(XWikiContext context) + { + return (Map) context.get(JSON_ANSWER_KEY); + } + @Override public boolean action(XWikiContext context) throws XWikiException { @@ -529,13 +567,7 @@ public boolean action(XWikiContext context) throws XWikiException // forward to view if (isAjaxRequest) { - Map jsonAnswer = new LinkedHashMap<>(); - Version newVersion = (Version) context.get(SAVED_OBJECT_VERSION_KEY); - jsonAnswer.put("newVersion", newVersion.toString()); - if ("true".equals(context.get(MERGED_DOCUMENTS))) { - jsonAnswer.put("mergedDocument", "true"); - } - answerJSON(context, HttpStatus.SC_OK, jsonAnswer); + answerJSON(context, HttpStatus.SC_OK, getJSONAnswer(context)); } else { sendRedirect(context.getResponse(), Utils.getRedirect("view", context)); } diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/web/SaveAndContinueAction.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/web/SaveAndContinueAction.java index 70da4c3ae8b..60ac16e8d70 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/web/SaveAndContinueAction.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/web/SaveAndContinueAction.java @@ -20,7 +20,6 @@ package com.xpn.xwiki.web; import java.io.IOException; -import java.util.LinkedHashMap; import java.util.Map; import javax.inject.Inject; @@ -32,7 +31,6 @@ import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.suigeneris.jrcs.rcs.Version; import org.xwiki.component.annotation.Component; import com.xpn.xwiki.XWikiContext; @@ -53,16 +51,6 @@ public class SaveAndContinueAction extends XWikiAction /** Key for storing the wrapped action in the context. */ private static final String WRAPPED_ACTION_CONTEXT_KEY = "SaveAndContinueAction.wrappedAction"; - /** - * The key to retrieve the saved object version from the context. - */ - private static final String SAVED_OBJECT_VERSION_KEY = "SaveAction.savedObjectVersion"; - - /** - * The context key to know if a document has been merged for saving it. - */ - private static final String MERGED_DOCUMENTS = "SaveAction.mergedDocuments"; - /** * The default context value to put with {@link #MERGED_DOCUMENTS} key. */ @@ -175,15 +163,10 @@ public boolean action(XWikiContext context) throws XWikiException // If this is an ajax request, no need to redirect. if (isAjaxRequest) { - Version newVersion = (Version) context.get(SAVED_OBJECT_VERSION_KEY); - - // in case of property update, SaveAction has not been called, so we don't get the new version. - if (newVersion != null) { - Map jsonAnswer = new LinkedHashMap<>(); - jsonAnswer.put("newVersion", newVersion.toString()); - if (MERGED_DOCUMENTS_VALUE.equals(context.get(MERGED_DOCUMENTS))) { - jsonAnswer.put("mergedDocument", MERGED_DOCUMENTS_VALUE); - } + @SuppressWarnings("unchecked") + Map jsonAnswer = (Map) context.get(SaveAction.JSON_ANSWER_KEY); + // In case of property update, SaveAction is not called, so there's no JSON answer. + if (jsonAnswer != null) { answerJSON(context, HttpStatus.SC_OK, jsonAnswer); } else { context.getResponse().setStatus(HttpServletResponse.SC_NO_CONTENT); diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/web/SaveActionTest.java b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/web/SaveActionTest.java index 32205a86d23..d9ba0b11844 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/web/SaveActionTest.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/web/SaveActionTest.java @@ -33,6 +33,7 @@ import java.util.Collections; import java.util.Date; import java.util.List; +import java.util.Map; import java.util.Optional; import javax.inject.Named; @@ -197,7 +198,7 @@ void validSave() throws Exception when(mockForm.getTemplate()).thenReturn(""); assertFalse(saveAction.save(this.context)); - assertEquals(new Version("1.2"), this.context.get("SaveAction.savedObjectVersion")); + assertEquals(Map.of("newVersion", "1.2"), saveAction.getJSONAnswer(context)); verify(mockAuthors).setOriginalMetadataAuthor(this.currentUserReference); verify(mockAuthors).setEffectiveMetadataAuthor(this.effectiveAuthor); @@ -220,7 +221,7 @@ void validSaveNewTranslation() throws Exception when(mockRequest.getParameter("previousVersion")).thenReturn("1.1"); when(mockRequest.getParameter("isNew")).thenReturn("true"); assertFalse(saveAction.save(this.context)); - assertEquals(new Version("1.1"), this.context.get("SaveAction.savedObjectVersion")); + assertEquals(Map.of("newVersion", "1.1"), saveAction.getJSONAnswer(context)); verify(this.xWiki).checkSavingDocument(eq(USER_REFERENCE), any(XWikiDocument.class), eq(""), eq(false), eq(this.context)); verify(this.xWiki).saveDocument(any(XWikiDocument.class), eq(""), eq(false), eq(this.context)); @@ -244,7 +245,7 @@ void validSaveOldTranslation() throws Exception when(mockClonedDocument.getRCSVersion()).thenReturn(new Version("1.4")); when(mockClonedDocument.getComment()).thenReturn("My Changes"); assertFalse(saveAction.save(this.context)); - assertEquals(new Version("1.4"), this.context.get("SaveAction.savedObjectVersion")); + assertEquals(Map.of("newVersion", "1.4"), saveAction.getJSONAnswer(context)); verify(this.xWiki).checkSavingDocument(USER_REFERENCE, mockClonedDocument, "My Changes", false, this.context); verify(this.xWiki).saveDocument(mockClonedDocument, "My Changes", false, this.context); } @@ -275,7 +276,7 @@ void validSaveRequestImageUploadAndConflictCheck() throws Exception when(mockDocument.getObjectDiff("1.1", "1.2", context)).thenReturn(Collections.emptyList()); assertFalse(saveAction.save(this.context)); - assertEquals(new Version("1.2"), this.context.get("SaveAction.savedObjectVersion")); + assertEquals(Map.of("newVersion", "1.2"), saveAction.getJSONAnswer(context)); verify(mockAuthors).setOriginalMetadataAuthor(this.currentUserReference); verify(mockAuthors).setEffectiveMetadataAuthor(this.effectiveAuthor); @@ -288,6 +289,7 @@ void validSaveRequestImageUploadAndConflictCheck() throws Exception @Test void saveFromTemplate() throws Exception { + when(mockClonedDocument.getRCSVersion()).thenReturn(new Version("3.2")); when(this.mockForm.getTemplate()).thenReturn("TemplateSpace.TemplateDocument"); DocumentReference templateReference = new DocumentReference(context.getWikiId(), "TemplateSpace", "TemplateDocument"); @@ -329,6 +331,7 @@ void saveSectionWithAttachmentUpload() throws Exception String comment = "Some comment"; when(sectionDoc.getComment()).thenReturn(comment); when(sectionDoc.isMinorEdit()).thenReturn(true); + when(mockClonedDocument.getRCSVersion()).thenReturn(new Version("4.1")); assertFalse(this.saveAction.save(this.context)); diff --git a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/Coeditor.java b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/CoeditorElement.java similarity index 97% rename from xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/Coeditor.java rename to xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/CoeditorElement.java index 96356648eb5..8bdaadb86f2 100644 --- a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/Coeditor.java +++ b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/CoeditorElement.java @@ -30,7 +30,7 @@ * @since 16.10.6 * @since 17.3.0RC1 */ -public class Coeditor extends BaseElement +public class CoeditorElement extends BaseElement { private WebElement container; @@ -39,7 +39,7 @@ public class Coeditor extends BaseElement * * @param container the WebElement used to display the coeditor */ - public Coeditor(WebElement container) + public CoeditorElement(WebElement container) { this.container = container; } diff --git a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/CoeditorsDropdown.java b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/CoeditorsDropdown.java index a504dc519ff..bc79049c1a3 100644 --- a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/CoeditorsDropdown.java +++ b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/CoeditorsDropdown.java @@ -85,19 +85,19 @@ private WebElement getToggle() /** * @return the list of users that are currently editing the page in realtime */ - public List getCoeditors() + public List getCoeditors() { return getDriver().findElements(By.cssSelector(".realtime-users-dropdown .realtime-user")).stream() - .map(Coeditor::new).toList(); + .map(CoeditorElement::new).toList(); } /** * @param coeditorId the coeditor identifier * @return the coeditor with the specified identifier */ - public Coeditor getCoeditor(String coeditorId) + public CoeditorElement getCoeditor(String coeditorId) { - return new Coeditor( + return new CoeditorElement( getDriver().findElement(By.cssSelector(".realtime-users-dropdown[data-id='" + coeditorId + "']"))); } } diff --git a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/HistoryDropdown.java b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/HistoryDropdown.java new file mode 100644 index 00000000000..0b2d8bc054e --- /dev/null +++ b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/HistoryDropdown.java @@ -0,0 +1,104 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.realtime.test.po; + +import java.util.List; + +import org.openqa.selenium.By; +import org.openqa.selenium.Keys; +import org.openqa.selenium.WebElement; +import org.openqa.selenium.support.ui.ExpectedConditions; +import org.xwiki.test.ui.po.BaseElement; + +/** + * The dropdown that lists recent versions of the edited document. + * + * @version $Id$ + * @since 17.9.0 + * @since 17.4.6 + * @since 16.10.13 + */ +public class HistoryDropdown extends BaseElement +{ + private static final By DROPDOWN_MENU = By.cssSelector(".realtime-versions.dropdown-menu"); + + /** + * Clicks on the dropdown toggle to open the dropdown. + * + * @return this instance + */ + public HistoryDropdown open() + { + getToggle().click(); + getDriver().waitUntilElementIsVisible(DROPDOWN_MENU); + return this; + } + + /** + * Uses the ESCAPE key to close the dropdown. + * + * @return this instance + */ + public HistoryDropdown close() + { + getToggle().sendKeys(Keys.ESCAPE); + getDriver().waitUntilElementDisappears(DROPDOWN_MENU); + return this; + } + + private WebElement getToggle() + { + return getDriver().findElement(By.cssSelector(".realtime-action-history.dropdown-toggle")); + } + + /** + * @return the document versions listed in the history dropdown + */ + public List getVersions() + { + return getDriver().findElements(By.cssSelector(".realtime-versions .realtime-version")).stream() + .map(VersionElement::new).toList(); + } + + /** + * Waits for the version with the given version number to be listed in the history dropdown. + * + * @param versionNumber the version number to wait for + * @return this instance + */ + public HistoryDropdown waitForVersion(String versionNumber) + { + getDriver().waitUntilCondition(ExpectedConditions.presenceOfElementLocated( + By.cssSelector(".realtime-version[data-version*=\"\\\"" + versionNumber + "\\\"\"]"))); + return this; + } + + /** + * Clicks on the "Summarize Changes" entry from the history dropdown and waits for the corresponding modal to be + * displayed. + * + * @return the modal used to review and summarize the changes + */ + public SummaryModal summarizeChanges() + { + getDriver().findElement(By.cssSelector(".realtime-versions .realtime-action-summarize")).click(); + return new SummaryModal(); + } +} diff --git a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/RealtimeEditToolbar.java b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/RealtimeEditToolbar.java index ec3faeb5b34..2783539403d 100644 --- a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/RealtimeEditToolbar.java +++ b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/RealtimeEditToolbar.java @@ -126,9 +126,7 @@ public SummaryModal clickSummarizeAndDone() { openDoneDropdown(); getDriver().findElement(By.cssSelector(".realtime-edit-toolbar .realtime-action-summarize")).click(); - SummaryModal summaryModal = new SummaryModal(); - getDriver().waitUntilCondition(it -> summaryModal.isDisplayed()); - return summaryModal; + return new SummaryModal(); } /** @@ -142,10 +140,10 @@ public String getUserId() /** * @return the list of coeditors listed directly on the toolbar */ - public List getVisibleCoeditors() + public List getVisibleCoeditors() { return getDriver().findElements(By.cssSelector(".realtime-edit-toolbar .realtime-users .realtime-user")) - .stream().map(Coeditor::new).toList(); + .stream().map(CoeditorElement::new).toList(); } /** @@ -154,12 +152,12 @@ public List getVisibleCoeditors() * @param coeditorId the coeditor identifier * @return this instance */ - public Coeditor waitForCoeditor(String coeditorId) + public CoeditorElement waitForCoeditor(String coeditorId) { By coeditorSelector = By.cssSelector(".realtime-edit-toolbar .realtime-user[data-id='" + coeditorId + "']"); // The coeditor can be either displayed directly on the toolbar or hidden in the dropdown. getDriver().waitUntilCondition(ExpectedConditions.presenceOfElementLocated(coeditorSelector)); - return new Coeditor(getDriver().findElement(coeditorSelector)); + return new CoeditorElement(getDriver().findElement(coeditorSelector)); } /** @@ -168,7 +166,7 @@ public Coeditor waitForCoeditor(String coeditorId) */ public boolean isEditingAlone() { - List visibleCoeditors = getVisibleCoeditors(); + List visibleCoeditors = getVisibleCoeditors(); return visibleCoeditors.size() == 1 && visibleCoeditors.get(0).getId().equals(getUserId()); } @@ -239,15 +237,28 @@ public void sendSaveShortcutKey(boolean wait) public RealtimeEditToolbar waitForConcurrentEditingWarning() { String toggleSelector = "button.realtime-warning[data-toggle=\"popover\"]"; - getDriver().waitUntilElementIsVisible(By.cssSelector(toggleSelector)); String popoverSelector = toggleSelector + " + .popover"; - if (!getDriver().findElementsWithoutWaiting(By.cssSelector(popoverSelector)).isEmpty()) { - // The popover is displayed. Let's hide it as it can cover other UI elements. - WebElement toggle = getDriver().findElementWithoutWaiting(By.cssSelector(toggleSelector)); - // We have to click twice because the popover was displayed without focusing the toggle. - toggle.click(); - toggle.click(); - } + + // Wait for the popover to be fully displayed, because it uses a fade-in animation. + getDriver().waitUntilElementIsVisible(By.cssSelector(popoverSelector + ".fade.in")); + + // Hide the popover because it can cover other UI elements. + WebElement toggle = getDriver().findElementWithoutWaiting(By.cssSelector(toggleSelector)); + // We have to click twice because the popover was displayed without focusing the toggle. + toggle.click(); + toggle.click(); + + // Wait for the popover to be fully hidden, because it uses a fade-out animation. + getDriver().waitUntilCondition(ExpectedConditions.numberOfElementsToBe(By.cssSelector(popoverSelector), 0)); + return this; } + + /** + * @return the dropdown listing recent versions of the edited document, and the "Summarize Changes" action + */ + public HistoryDropdown getHistoryDropdown() + { + return new HistoryDropdown(); + } } diff --git a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/SummaryModal.java b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/SummaryModal.java index 9e6a0e4328a..8372afecc61 100644 --- a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/SummaryModal.java +++ b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/SummaryModal.java @@ -20,6 +20,7 @@ package org.xwiki.realtime.test.po; import org.openqa.selenium.By; +import org.openqa.selenium.WebElement; import org.xwiki.test.ui.po.BaseModal; /** @@ -35,16 +36,20 @@ public class SummaryModal extends BaseModal public SummaryModal() { super(By.id("realtime-changeSummaryModal")); + getDriver().waitUntilCondition(driver -> this.isDisplayed()); } /** * Fill the summary textarea with the given content. + * * @param summary the text to put in the summary textarea */ public void setSummary(String summary) { - getDriver().findElementWithoutWaiting(this.container, By.id("realtime-changeSummaryModal-summary")) - .sendKeys(summary); + WebElement textarea = + getDriver().findElementWithoutWaiting(this.container, By.id("realtime-changeSummaryModal-summary")); + textarea.clear(); + textarea.sendKeys(summary); } /** @@ -52,12 +57,14 @@ public void setSummary(String summary) */ public void toggleMinorEdit() { - getDriver().findElementWithoutWaiting(this.container, - By.cssSelector("input[type='checkbox'][name='minorChange']")).click(); + getDriver() + .findElementWithoutWaiting(this.container, By.cssSelector("input[type='checkbox'][name='minorChange']")) + .click(); } /** * Click on the save button and eventually wait for the saved success message. + * * @param waitSuccess if {@code true} wait for the "Saved" success message */ public void clickSave(boolean waitSuccess) diff --git a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/VersionElement.java b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/VersionElement.java new file mode 100644 index 00000000000..efe63939545 --- /dev/null +++ b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/VersionElement.java @@ -0,0 +1,71 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.realtime.test.po; + +import org.openqa.selenium.By; +import org.openqa.selenium.WebElement; +import org.xwiki.test.ui.po.BaseElement; + +/** + * Represents a document version listed in the history dropdown from the realtime editing toolbar. + * + * @version $Id$ + * @since 17.9.0 + * @since 17.4.6 + * @since 16.10.13 + */ +public class VersionElement extends BaseElement +{ + private WebElement container; + + /** + * Creates a new instance based on the given version element. + * + * @param container the WebElement used to display the version + */ + public VersionElement(WebElement container) + { + this.container = container; + } + + /** + * @return the author of this version + */ + public CoeditorElement getAuthor() + { + return new CoeditorElement(this.container.findElement(By.className("realtime-user"))); + } + + /** + * @return the version number + */ + public String getNumber() + { + return this.container.findElement(By.className("realtime-version-number")).getText(); + } + + /** + * @return the version date + */ + public String getDate() + { + return this.container.findElement(By.className("realtime-version-date")).getText(); + } +} diff --git a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-ui/src/main/resources/XWiki/Realtime/Configuration.xml b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-ui/src/main/resources/XWiki/Realtime/Configuration.xml index 1e76fc06b0a..429bda8df14 100644 --- a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-ui/src/main/resources/XWiki/Realtime/Configuration.xml +++ b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-ui/src/main/resources/XWiki/Realtime/Configuration.xml @@ -565,6 +565,7 @@ ul.realtime-versions > li.divider:last-child { <template id="realtime-edit-toolbar"> <div class="realtime-edit-toolbar" data-user-id=""> <div class="realtime-edit-toolbar-left"> + <input type="hidden" name="preventEmptyRevision" value="true" /> <div class="btn-group dropup"> <button type="button" diff --git a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-webjar/src/main/webjar/saver.js b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-webjar/src/main/webjar/saver.js index 47c3df4ab8b..53a1b81fe91 100644 --- a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-webjar/src/main/webjar/saver.js +++ b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-webjar/src/main/webjar/saver.js @@ -524,8 +524,8 @@ define('xwiki-realtime-saver', [ number: revision.version, date: new Date(revision.modified).getTime(), author: { - reference: this._getAbsoluteUserReference(revision.modifier), - name: revision.modifierName + reference: this._getAbsoluteUserReference(revision.author), + name: revision.authorName } }); }).catch(error => { @@ -665,7 +665,10 @@ define('xwiki-realtime-saver', [ } _afterSave({newVersion}) { - if (newVersion === '1.1') { + if (newVersion === xwikiDocument.version) { + // The version didn't change because the document hasn't been modified. + return; + } else if (newVersion === '1.1') { debug('Created document version 1.1'); } else { debug(`Version bumped from ${xwikiDocument.version} to ${newVersion}.`); diff --git a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-webjar/src/main/webjar/toolbar.js b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-webjar/src/main/webjar/toolbar.js index 4cfe629a3b0..e548ddde5f1 100644 --- a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-webjar/src/main/webjar/toolbar.js +++ b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-webjar/src/main/webjar/toolbar.js @@ -272,12 +272,23 @@ define('xwiki-realtime-toolbar', [ if (!this._lastReviewedVersion) { this._lastReviewedVersion = version.number; } - const versions = this._toolbar.querySelectorAll('.realtime-version'); + // Make sure we don't add the same version twice. This can happen if some of the previous versions were deleted + // and their version numbers are being reused. + const versions = [...this._toolbar.querySelectorAll('.realtime-version')].filter(existingVersion => { + if (JSON.parse(existingVersion.dataset.version).number === version.number) { + // The version element is wrapped in a list item element, that we need to remove as well. + existingVersion.parentElement.remove(); + return false; + } + return true; + }); + // Limit the number of versions shown in the dropdown. const limit = Number.parseInt(this._toolbar.querySelector('.realtime-versions').dataset.limit) || 5; if (versions.length >= limit) { - // The version element is wrapped in a list item element. + // The version element is wrapped in a list item element, that we need to remove as well. versions[0].parentElement.remove(); } + // Insert the new version. const versionWrapper = this._createVersionElement(version); this._toolbar.querySelector('.realtime-versions > .divider').before(versionWrapper); } diff --git a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-wysiwyg/xwiki-platform-realtime-wysiwyg-test/xwiki-platform-realtime-wysiwyg-test-docker/src/test/it/org/xwiki/realtime/wysiwyg/test/ui/RealtimeWYSIWYGEditorIT.java b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-wysiwyg/xwiki-platform-realtime-wysiwyg-test/xwiki-platform-realtime-wysiwyg-test-docker/src/test/it/org/xwiki/realtime/wysiwyg/test/ui/RealtimeWYSIWYGEditorIT.java index 0838bfafcbb..59c911c9f37 100644 --- a/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-wysiwyg/xwiki-platform-realtime-wysiwyg-test/xwiki-platform-realtime-wysiwyg-test-docker/src/test/it/org/xwiki/realtime/wysiwyg/test/ui/RealtimeWYSIWYGEditorIT.java +++ b/xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-wysiwyg/xwiki-platform-realtime-wysiwyg-test/xwiki-platform-realtime-wysiwyg-test-docker/src/test/it/org/xwiki/realtime/wysiwyg/test/ui/RealtimeWYSIWYGEditorIT.java @@ -51,11 +51,13 @@ import org.xwiki.model.reference.EntityReference; import org.xwiki.panels.test.po.DocumentInformationPanel; import org.xwiki.realtime.test.RealtimeTestUtils; -import org.xwiki.realtime.test.po.Coeditor; +import org.xwiki.realtime.test.po.CoeditorElement; +import org.xwiki.realtime.test.po.HistoryDropdown; import org.xwiki.realtime.test.po.RealtimeEditToolbar; import org.xwiki.realtime.test.po.RealtimeInplaceEditablePage; import org.xwiki.realtime.test.po.SaveStatus; import org.xwiki.realtime.test.po.SummaryModal; +import org.xwiki.realtime.test.po.VersionElement; import org.xwiki.realtime.wysiwyg.test.po.RealtimeCKEditor; import org.xwiki.realtime.wysiwyg.test.po.RealtimeRichTextAreaElement; import org.xwiki.realtime.wysiwyg.test.po.RealtimeRichTextAreaElement.CoeditorPosition; @@ -179,7 +181,7 @@ void editAlone(TestReference testReference, TestUtils setup) inplaceEditablePage.getToolbar().waitForSaveStatus(SaveStatus.UNSAVED); inplaceEditablePage.getToolbar().waitForSaveStatus(SaveStatus.SAVED); - inplaceEditablePage.done(); + setup.leaveEditMode(); assertEquals("zero\ntwo\nthree", inplaceEditablePage.getContent()); // edit again and test the summarize & done @@ -195,11 +197,11 @@ void editAlone(TestReference testReference, TestUtils setup) summaryModal.clickSave(true); inplaceEditablePage.waitForView(); assertEquals("zero\ntwo\nthree\nfour", inplaceEditablePage.getContent()); - HistoryPane historyPane = inplaceEditablePage.openHistoryDocExtraPane(); + HistoryPane historyPane = inplaceEditablePage.openHistoryDocExtraPane().showMinorEdits(); assertEquals(4, historyPane.getNumberOfVersions()); assertEquals("Summarize changes", historyPane.getCurrentVersionComment()); - // delete the page to test creation with summarize & done (regression test for XWIKI-23136) + // Delete the page to test creation with summarize & done (regression test for XWIKI-23136). setup.deletePage(testReference); editPage = RealtimeWYSIWYGEditPage.gotoPage(testReference); editor = editPage.getContenEditor(); @@ -253,7 +255,7 @@ void editWithSelf(TestReference testReference, TestUtils setup, MultiUserTestUti String secondCoeditorId = secondEditPage.getToolbar().getUserId(); // Verify the list of coeditors. - Coeditor self = secondEditPage.getToolbar().waitForCoeditor(firstCoeditorId); + CoeditorElement self = secondEditPage.getToolbar().waitForCoeditor(firstCoeditorId); assertTrue(self.isDisplayed()); // The name is not visible when the user is displayed directly on the toolbar. assertEquals("", self.getName()); @@ -1441,7 +1443,7 @@ void editPageTranslations(TestUtils setup, TestReference testReference, MultiUse assertEquals("Deutsch content", firstTextArea.getText()); assertEquals("John, superadmin", firstEditPage.getToolbar().getVisibleCoeditors().stream() - .map(Coeditor::getAvatarHint).reduce((a, b) -> a + ", " + b).get()); + .map(CoeditorElement::getAvatarHint).reduce((a, b) -> a + ", " + b).get()); } @Test @@ -2332,6 +2334,257 @@ void syntaxChange(TestUtils setup, TestReference testReference, MultiUserTestUti assertEquals("[[label>>||anchor=\"target\"]] after", source); } + @Test + @Order(25) + void preventEmptyRevisions(TestUtils setup, TestReference testReference, MultiUserTestUtils multiUserSetup) + { + // + // First Tab + // + + // Start fresh. + setup.loginAsSuperAdmin(); + setup.deletePage(testReference); + + // Edit the page in the first browser tab (standalone). + RealtimeWYSIWYGEditPage firstEditPage = RealtimeWYSIWYGEditPage.gotoPage(testReference); + + // There are no revisions yet. + HistoryDropdown historyDropdown = firstEditPage.getToolbar().getHistoryDropdown(); + assertTrue(historyDropdown.open().getVersions().isEmpty()); + + // Even if there is no change, the document is new so the Done button should create the first revision. + firstEditPage.clickDone(); + + // + // Second Tab + // + + String secondTabHandle = multiUserSetup.openNewBrowserTab(XWIKI_ALIAS); + loginAsJohn(setup); + + // Edit the page in the second browser tab (in-place). + RealtimeInplaceEditablePage inplaceEditablePage = RealtimeInplaceEditablePage.gotoPage(testReference); + inplaceEditablePage.editInplace(); + RealtimeCKEditor secondEditor = new RealtimeCKEditor(); + RealtimeRichTextAreaElement secondTextArea = secondEditor.getRichTextArea(); + + // Verify that the history dropdown contains only one entry (the initial revision). + List versions = historyDropdown.open().getVersions(); + assertEquals(1, versions.size()); + assertEquals("1.1", versions.get(0).getNumber()); + assertEquals("Su", versions.get(0).getAuthor().getAbbreviation()); + + // Try to save without making any changes. Even if the author is different, no new revision should be created. + inplaceEditablePage.getToolbar().sendSaveShortcutKey(); + + // Verify that the history dropdown still contains only one entry (the initial revision). + versions = historyDropdown.open().getVersions(); + assertEquals(1, versions.size()); + assertEquals("1.1", versions.get(0).getNumber()); + assertEquals("Su", versions.get(0).getAuthor().getAbbreviation()); + + // + // First Tab + // + + multiUserSetup.switchToBrowserTab(multiUserSetup.getFirstTabHandle()); + // Join the realtime collaboration. + firstEditPage = RealtimeWYSIWYGEditPage.gotoPage(testReference); + RealtimeCKEditor firstEditor = firstEditPage.getContenEditor(); + RealtimeRichTextAreaElement firstTextArea = firstEditor.getRichTextArea(); + + // + // Second Tab + // + + multiUserSetup.switchToBrowserTab(secondTabHandle); + secondTextArea.sendKeys("one"); + inplaceEditablePage.getToolbar().sendSaveShortcutKey(); + + // Verify that the history dropdown contains two versions. + versions = historyDropdown.open().getVersions(); + assertEquals(2, versions.size()); + assertEquals("1.1", versions.get(0).getNumber()); + assertEquals("Su", versions.get(0).getAuthor().getAbbreviation()); + assertEquals("1.2", versions.get(1).getNumber()); + assertEquals("Jo", versions.get(1).getAuthor().getAbbreviation()); + historyDropdown.close(); + + // + // First Tab + // + + multiUserSetup.switchToBrowserTab(multiUserSetup.getFirstTabHandle()); + firstTextArea.waitUntilTextContains("one"); + + // Verify that the new version was propagated. + versions = historyDropdown.waitForVersion("1.2").open().getVersions(); + assertEquals(2, versions.size()); + assertEquals("1.1", versions.get(0).getNumber()); + assertEquals("Su", versions.get(0).getAuthor().getAbbreviation()); + assertEquals("1.2", versions.get(1).getNumber()); + assertEquals("Jo", versions.get(1).getAuthor().getAbbreviation()); + + // Try to save without making any changes. No new revision should be created. + firstEditPage.getToolbar().sendSaveShortcutKey(); + + // Verify that the history dropdown still contains only two entries. + versions = historyDropdown.open().getVersions(); + assertEquals(2, versions.size()); + + firstTextArea.sendKeys(Keys.END, " two"); + assertEquals("one two", firstEditPage.clickDone().getContent()); + + // + // Second Tab + // + + multiUserSetup.switchToBrowserTab(secondTabHandle); + secondTextArea.waitUntilTextContains("two"); + + // Verify that the new version was propagated. + versions = historyDropdown.waitForVersion("2.1").open().getVersions(); + assertEquals(3, versions.size()); + assertEquals("1.1", versions.get(0).getNumber()); + assertEquals("Su", versions.get(0).getAuthor().getAbbreviation()); + assertEquals("1.2", versions.get(1).getNumber()); + assertEquals("Jo", versions.get(1).getAuthor().getAbbreviation()); + assertEquals("2.1", versions.get(2).getNumber()); + assertEquals("Su", versions.get(2).getAuthor().getAbbreviation()); + + // Leave the editing session without making any changes. No new revision should be created. + HistoryPane historyPane = inplaceEditablePage.done().openHistoryDocExtraPane().showMinorEdits(); + assertEquals(3, historyPane.getNumberOfVersions()); + assertEquals("2.1", historyPane.getCurrentVersion()); + assertEquals("superadmin", historyPane.getCurrentAuthor()); + + // + // First Tab + // + + // Edit again to check that we can force a new empty revision by specifying a version summary. + multiUserSetup.switchToBrowserTab(multiUserSetup.getFirstTabHandle()); + firstEditPage = RealtimeWYSIWYGEditPage.gotoPage(testReference); + + assertEquals(1, historyDropdown.open().getVersions().size()); + + SummaryModal summaryModal = historyDropdown.summarizeChanges(); + summaryModal.setSummary("An empty revision"); + summaryModal.clickSave(true); + + versions = historyDropdown.open().getVersions(); + assertEquals(2, versions.size()); + assertEquals("2.2", versions.get(1).getNumber()); + assertEquals("Su", versions.get(1).getAuthor().getAbbreviation()); + + // Try again, this time without a summary. + summaryModal = historyDropdown.summarizeChanges(); + summaryModal.setSummary(""); + summaryModal.clickSave(false); + firstEditPage.getToolbar().waitForSaveStatus(SaveStatus.SAVED); + + assertEquals(2, historyDropdown.open().getVersions().size()); + + // Leave the editing session with a summary. This should create a new revision. + summaryModal = firstEditPage.getToolbar().clickSummarizeAndDone(); + summaryModal.setSummary("Another empty revision"); + summaryModal.clickSave(true); + + historyPane = new ViewPage().openHistoryDocExtraPane().showMinorEdits(); + assertEquals(5, historyPane.getNumberOfVersions()); + assertEquals("3.1", historyPane.getCurrentVersion()); + assertEquals("superadmin", historyPane.getCurrentAuthor()); + assertEquals("Another empty revision", historyPane.getCurrentVersionComment()); + + // + // Second Tab + // + + multiUserSetup.switchToBrowserTab(secondTabHandle); + // Edit again to verify that the latest version is returned even when there are no changes. The plan is to save + // the same document state with both users, before making any changes and after making the same changes. + inplaceEditablePage = RealtimeInplaceEditablePage.gotoPage(testReference); + inplaceEditablePage.editInplace(); + secondEditor = new RealtimeCKEditor(); + secondTextArea = secondEditor.getRichTextArea(); + + // + // First Tab + // + + multiUserSetup.switchToBrowserTab(multiUserSetup.getFirstTabHandle()); + firstEditPage = RealtimeWYSIWYGEditPage.gotoPage(testReference); + firstEditor = firstEditPage.getContenEditor(); + firstTextArea = firstEditor.getRichTextArea(); + + // Verify that there is a single revision listed in the history dropdown. + versions = historyDropdown.open().getVersions(); + assertEquals(1, versions.size()); + assertEquals("3.1", versions.get(0).getNumber()); + assertEquals("Su", versions.get(0).getAuthor().getAbbreviation()); + + // + // Second Tab + // + + multiUserSetup.switchToBrowserTab(secondTabHandle); + // Focus the editing area to get the floating toolbar. + secondTextArea.click(); + secondEditor.getToolBar().toggleSourceMode(); + // Close the concurrent editing warning popover because it may cover other UI elements (like the Save button if + // the window width is too small for the toolbar to fit in a single line). + inplaceEditablePage.getToolbar().waitForConcurrentEditingWarning(); + // Save without making any changes. + inplaceEditablePage.save(); + + // + // First Tab + // + + multiUserSetup.switchToBrowserTab(multiUserSetup.getFirstTabHandle()); + // Save without making any changes. + firstEditPage.getToolbar().sendSaveShortcutKey(); + // Verify that we have two revisions listed in the history dropdown. + versions = historyDropdown.open().getVersions(); + assertEquals(2, versions.size()); + assertEquals("3.2", versions.get(1).getNumber()); + // The save action doesn't return the latest revision author so we assume the current user is the author. + assertEquals("Su", versions.get(1).getAuthor().getAbbreviation()); + + // + // Second Tab + // + + multiUserSetup.switchToBrowserTab(secondTabHandle); + // Make some changes and save. We'll do the same changes in the first tab afterwards. + secondEditor.getSourceTextArea().sendKeys(Keys.END, " three"); + inplaceEditablePage.save(); + + // + // First Tab + // + + multiUserSetup.switchToBrowserTab(multiUserSetup.getFirstTabHandle()); + // Make the same changes and save. + firstTextArea.sendKeys(Keys.END, " three"); + firstEditPage.getToolbar().sendSaveShortcutKey(); + // Verify that we have three revisions listed in the history dropdown. + versions = historyDropdown.open().getVersions(); + assertEquals(3, versions.size()); + assertEquals("3.3", versions.get(2).getNumber()); + // Again, the current user is not the real author of the new revision but the save action doesn't help us. + assertEquals("Su", versions.get(2).getAuthor().getAbbreviation()); + + // Make some more changes to verify we don't get a merge conflict. + firstTextArea.clear(); + firstTextArea.sendKeys("final"); + historyPane = firstEditPage.clickDone().openHistoryDocExtraPane().showMinorEdits(); + assertEquals(8, historyPane.getNumberOfVersions()); + assertEquals("4.1", historyPane.getCurrentVersion()); + assertEquals("superadmin", historyPane.getCurrentAuthor()); + } + private void setMultiLingual(TestUtils setup, boolean isMultiLingual, String... supportedLanguages) { AdministrationPage adminPage = AdministrationPage.gotoPage(); diff --git a/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war/src/main/webapp/resources/js/xwiki/actionbuttons/actionButtons.js b/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war/src/main/webapp/resources/js/xwiki/actionbuttons/actionButtons.js index 0c85b244f7a..453dfb477e7 100644 --- a/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war/src/main/webapp/resources/js/xwiki/actionbuttons/actionButtons.js +++ b/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war/src/main/webapp/resources/js/xwiki/actionbuttons/actionButtons.js @@ -23,24 +23,15 @@ var XWiki = (function(XWiki) { // Start XWiki augmentation. - var actionButtons = XWiki.actionButtons = XWiki.actionButtons || {}; - - // we need to handle the creation of document - var currentDocument; - var editingVersionDateField = $('editingVersionDate'); - var previousVersionField = $('previousVersion'); - var isNewField = $('isNew'); - - var refreshVersion = function (event) { - if (currentDocument.equals(event.memo.documentReference)) { - if (previousVersionField) { - previousVersionField.setValue(event.memo.version); - } - if (isNewField) { - isNewField.setValue(false); - } + const actionButtons = XWiki.actionButtons = XWiki.actionButtons || {}; + + function refreshVersion(event) { + if (XWiki.currentDocument.documentReference.equals(event.memo.documentReference)) { + $('previousVersion')?.setValue(event.memo.version); + $('editingVersionDate')?.setValue(Date.now()); + $('isNew')?.setValue(false); } - }; + } /** * Allow custom validation messages to be set on the validated field usin data attributes. @@ -372,22 +363,24 @@ var XWiki = (function(XWiki) { $$('input[name=mergeChoices]').forEach(function (item) {item.remove();}); $$('input[name=customChoices]').forEach(function (item) {item.remove();}); - var hasBeenSaved = false; + let hasBeenSaved = false; if (state.isCreateFromTemplate) { // We might have a responseJSON containing other information than links, if the template cannot be accessed. - if (response.responseJSON && response.responseJSON.links) { + if (response.responseJSON?.links) { // Start the progress display. this.getStatus(response.responseJSON.links[0].href, state); } else { this.progressBox.hide(); this.savingBox.replace(this.savedBox); - // in such case the page is saved, so we'll need to maybe redirect + // In such case the page is saved, so we'll need to maybe redirect. hasBeenSaved = true; } } else { this.progressBox.hide(); - if (response.responseJSON && response.responseJSON.mergedDocument) { + if (response.responseJSON?.mergedDocument) { this.savingBox.replace(this.savedWithMergeBox); + } else if (response.responseJSON?.noChanges) { + this.savingBox.hide(); } else { this.savingBox.replace(this.savedBox); } @@ -401,23 +394,18 @@ var XWiki = (function(XWiki) { } } - if (response.responseJSON && response.responseJSON.newVersion) { - // update the version + if (response.responseJSON?.newVersion) { + // Update the version. require(['xwiki-meta'], function (xm) { xm.setVersion(response.responseJSON.newVersion); }); - - // We only update this field since the other ones are updated by the callback of setVersion. - if (editingVersionDateField) { - editingVersionDateField.setValue(new Date().getTime()); - } } - // Announce that the document has been saved + // Announce that the document has been saved. state.saveButton.fire("xwiki:document:saved", response.responseJSON); // If documents have been merged we need to reload to get latest saved version. - if (response.responseJSON && response.responseJSON.mergedDocument) { + if (response.responseJSON?.mergedDocument) { this.reloadEditor(); } }, @@ -806,10 +794,6 @@ var XWiki = (function(XWiki) { }); function init() { - require(['xwiki-meta'], function (xm) { - currentDocument = xm.documentReference; - }); - new actionButtons.EditActions(); new actionButtons.AjaxSaveAndContinue(); return true; diff --git a/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war/src/main/webapp/resources/js/xwiki/meta.js b/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war/src/main/webapp/resources/js/xwiki/meta.js index b9399292649..693170ffbc2 100644 --- a/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war/src/main/webapp/resources/js/xwiki/meta.js +++ b/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war/src/main/webapp/resources/js/xwiki/meta.js @@ -74,11 +74,13 @@ define(['jquery', 'xwiki-entityReference', 'xwiki-events-bridge'], function($, X } setVersion(newVersion) { - this.version = newVersion; - $(document).trigger('xwiki:document:changeVersion', { - 'version': this.version, - 'documentReference': this.documentReference - }); + if (newVersion !== this.version) { + this.version = newVersion; + $(document).trigger('xwiki:document:changeVersion', { + 'version': this.version, + 'documentReference': this.documentReference + }); + } } /**