Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XWIKI-22787: Pinned Pages are lost on space move for non admin user #3886

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

XWIKI-22787: Pinned Pages are lost on space move for non admin user #3886

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0608ada
XWIKI-22787: Pinned Pages are lost on space move for non admin user
surli Feb 10, 2025
dc2a2b9
XWIKI-22787: Pinned Pages are lost on space move for non admin user
surli Feb 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 30 additions & 31 deletions ...factoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractCopyOrMoveJob.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@

import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.function.BiConsumer;
import java.util.stream.Collectors;

import org.xwiki.model.EntityType;
import org.xwiki.model.reference.DocumentReference;
Expand Down Expand Up @@ -309,14 +307,28 @@ protected void process(final SpaceReference source, final SpaceReference destina

protected boolean checkAllRights(DocumentReference oldReference, DocumentReference newReference) throws Exception
{
if (!hasAccess(Right.VIEW, oldReference)) {
this.logger.error("You don't have sufficient permissions over the source document [{}].", oldReference);
return false;
} else if (!hasAccess(Right.EDIT, newReference)
|| (this.modelBridge.exists(newReference) && !hasAccess(Right.DELETE, newReference))) {
this.logger.error("You don't have sufficient permissions over the destination document [{}].",
newReference);
return false;
boolean isWebPreferences =
isSpacePreferencesReference(oldReference) && isSpacePreferencesReference(newReference);
if (!isWebPreferences) {
if (!hasAccess(Right.VIEW, oldReference)) {
this.logger.error("You don't have sufficient permissions over the source document [{}].", oldReference);
return false;
} else if (!hasAccess(Right.EDIT, newReference)
|| (this.modelBridge.exists(newReference) && !hasAccess(Right.DELETE, newReference)))
{
this.logger.error("You don't have sufficient permissions over the destination document [{}].",
newReference);
return false;
}
} else {
DocumentReference newHomeReference = getSpaceHomeReference(newReference);
DocumentReference oldHomeReference = getSpaceHomeReference(oldReference);
if (!this.concernedEntities.containsKey(oldHomeReference) || !hasAccess(Right.EDIT, newHomeReference)) {
this.logger.error("You don't have sufficient permissions over the home document of WebPreferences "
+ "[{}].",
newReference);
return false;
}
}
return true;
}
Expand Down Expand Up @@ -352,14 +364,14 @@ protected boolean copyOrMove(DocumentReference oldReference, DocumentReference n

// Step 2: Delete the destination document if needed.
this.progressManager.startStep(this);
if (this.modelBridge.exists(newReference)) {
if (this.request.isInteractive() && !this.modelBridge.canOverwriteSilently(newReference)
&& !confirmOverwrite(oldReference, newReference)) {
this.logger.warn(
"Skipping [{}] because [{}] already exists and the user doesn't want to overwrite it.",
oldReference, newReference);
return false;
}
if (this.modelBridge.exists(newReference)
&& this.request.isInteractive()
&& !this.modelBridge.canOverwriteSilently(newReference)
&& !confirmOverwrite(oldReference, newReference)) {
this.logger.warn(
"Skipping [{}] because [{}] already exists and the user doesn't want to overwrite it.",
oldReference, newReference);
return false;
}
this.progressManager.endStep(this);

Expand Down Expand Up @@ -416,19 +428,6 @@ protected EntityReference getCommonParent()
return getCommonParent(entityReferences);
}

/**
* @return the list of references that have been selected to be refactored.
* @since 16.10.0RC1
*/
public Map<EntityReference, EntityReference> getSelectedEntities()
{
return this.concernedEntities.values().stream()
.filter(EntitySelection::isSelected)
.filter(entity -> entity.getTargetEntityReference().isPresent())
.collect(Collectors.toMap(EntitySelection::getEntityReference,
entity -> entity.getTargetEntityReference().get()));
}

/**
* Atomic operation to perform: should be a rename for Rename/Move and copy for Copy.
* @param source the source reference
Expand Down
12 changes: 9 additions & 3 deletions ...m-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractEntityJob.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ public interface Visitor<T>
void visit(T node);
}

private static final JobGroupPath ROOT_GROUP = new JobGroupPath(RefactoringJobs.GROUP, null);
protected static final String PREFERENCES_DOCUMENT_NAME = "WebPreferences";

private static final String PREFERENCES_DOCUMENT_NAME = "WebPreferences";
private static final JobGroupPath ROOT_GROUP = new JobGroupPath(RefactoringJobs.GROUP, null);

/**
* The component used to access the XWiki model and to perform low level operations on it.
Expand Down Expand Up @@ -261,7 +261,13 @@ protected boolean isSpaceHomeReference(DocumentReference documentReference)
.equals(this.defaultEntityReferenceProvider.getDefaultReference(documentReference.getType()).getName());
}

private boolean isSpacePreferencesReference(EntityReference entityReference)
protected DocumentReference getSpaceHomeReference(DocumentReference reference)
{
String referenceName = this.defaultEntityReferenceProvider.getDefaultReference(EntityType.DOCUMENT).getName();
return new DocumentReference(referenceName, reference.getLastSpaceReference());
}

protected boolean isSpacePreferencesReference(EntityReference entityReference)
{
return entityReference.getType() == EntityType.DOCUMENT
&& PREFERENCES_DOCUMENT_NAME.equals(entityReference.getName());
Expand Down
16 changes: 16 additions & 0 deletions ...ing-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractEntityJobWithChecks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.stream.Collectors;

import org.xwiki.bridge.event.DocumentsDeletingEvent;
import org.xwiki.model.reference.DocumentReference;
Expand Down Expand Up @@ -63,13 +64,28 @@ protected void runInternal() throws Exception
// Process
progressManager.startStep(this);
setContextUser();

// FIXME: this should probably be the selected entities only...
process(entityReferences);
}
} finally {
progressManager.popLevelProgress(this);
}
}

/**
* @return the list of references that have been selected to be refactored.
* @since 16.10.0RC1
*/
public Map<EntityReference, EntityReference> getSelectedEntities()
{
return this.concernedEntities.values().stream()
.filter(EntitySelection::isSelected)
.filter(entity -> entity.getTargetEntityReference().isPresent())
.collect(Collectors.toMap(EntitySelection::getEntityReference,
entity -> entity.getTargetEntityReference().get()));
}

protected void getEntities(Collection<EntityReference> entityReferences)
{
this.progressManager.pushLevelProgress(entityReferences.size(), this);
Expand Down
13 changes: 12 additions & 1 deletion ...ki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/MoveJob.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,18 @@ protected void getEntities(Collection<EntityReference> entityReferences)
@Override
protected boolean checkAllRights(DocumentReference oldReference, DocumentReference newReference) throws Exception
{
if (!hasAccess(Right.DELETE, oldReference)) {
boolean isWebPreferences =
isSpacePreferencesReference(oldReference) && isSpacePreferencesReference(newReference);
if (isWebPreferences) {
DocumentReference oldHomeReference = getSpaceHomeReference(oldReference);
if (!this.concernedEntities.containsKey(oldHomeReference) || !hasAccess(Right.DELETE, oldHomeReference)) {
this.logger.error("You don't have sufficient permissions over the home document of WebPreferences "
+ "[{}].",
newReference);
return false;
}
return super.checkAllRights(oldReference, newReference);
} else if (!hasAccess(Right.DELETE, oldReference)) {
// The move operation is implemented as Copy + Delete.
this.logger.error("You are not allowed to delete [{}].", oldReference);
return false;
Expand Down
1 change: 1 addition & 0 deletions ...latform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/job/MoveJobTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,7 @@ void moveSpaceHomeDeep() throws Throwable
DocumentReference spaceHome = new DocumentReference("chess", List.of("A", "B", "C"), "WebHome");
DocumentReference docFromSpace = new DocumentReference("X", spaceHome.getLastSpaceReference());
DocumentReference otherDocFromSpace = new DocumentReference("Y", spaceHome.getLastSpaceReference());
DocumentReference prefFromSpace = new DocumentReference("WebPreferences", spaceHome.getLastSpaceReference());
when(this.modelBridge.getDocumentReferences(spaceHome.getLastSpaceReference()))
.thenReturn(List.of(spaceHome, docFromSpace, otherDocFromSpace));
when(this.modelBridge.exists(spaceHome)).thenReturn(false);
Expand Down