Skip to content

Commit

Permalink
BUGFIX: Ensure for now that the api does not allow two personal works…
Browse files Browse the repository at this point in the history
…paces for one user

we might change the concepts later and add a mapping to the "current" users workspace in their session but for now we dont need the complexity
  • Loading branch information
mhsdesign committed Dec 9, 2024
1 parent 099274a commit 31473fb
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -352,10 +352,9 @@ public function findPrimaryWorkspaceNameForUser(ContentRepositoryId $contentRepo
content_repository_id = :contentRepositoryId
AND classification = :personalWorkspaceClassification
AND owner_user_id = :userId
-- TODO introduce better deterministic selection of "primary" workspace if multiple exist
ORDER BY workspace_name
LIMIT 1
SQL;
// We don't order the results and return the first matching workspace.
// In case multiple exist - which is not desired currently - and in the happy path not possible via the api (race conditions aside) - the order is not defined.
$workspaceName = $this->dbal->fetchOne($query, [
'contentRepositoryId' => $contentRepositoryId->value,
'personalWorkspaceClassification' => WorkspaceClassification::PERSONAL->value,
Expand Down
12 changes: 7 additions & 5 deletions Neos.Neos/Classes/Domain/Service/WorkspaceService.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,13 @@ public function setWorkspaceDescription(ContentRepositoryId $contentRepositoryId
}

/**
* Retrieve the first personal workspace for the specified user
*
* NOTE: Conventionally each user has only a single personal workspace but in case multiple exist the first is returned
* Retrieve the personal workspace for the specified user, if no workspace exist an exception is thrown.
*/
public function getPersonalWorkspaceForUser(ContentRepositoryId $contentRepositoryId, UserId $userId): Workspace
{
$workspaceName = $this->metadataAndRoleRepository->findPrimaryWorkspaceNameForUser($contentRepositoryId, $userId);
if ($workspaceName === null) {
throw new \RuntimeException(sprintf('No workspace is assigned to the user with id "%s")', $userId->value), 1718293801);
throw new \RuntimeException(sprintf('No workspace is assigned to the user with id "%s")', $userId->value), 1733755300);
}
return $this->requireWorkspace($contentRepositoryId, $workspaceName);
}
Expand Down Expand Up @@ -141,10 +139,14 @@ public function createRootWorkspace(ContentRepositoryId $contentRepositoryId, Wo
}

/**
* Create a new, personal, workspace for the specified user
* Create a new, personal, workspace for the specified user (fails if the user already owns a workspace)
*/
public function createPersonalWorkspace(ContentRepositoryId $contentRepositoryId, WorkspaceName $workspaceName, WorkspaceTitle $title, WorkspaceDescription $description, WorkspaceName $baseWorkspaceName, UserId $ownerId): void
{
$existingUserWorkspace = $this->metadataAndRoleRepository->findPrimaryWorkspaceNameForUser($contentRepositoryId, $ownerId);
if ($existingUserWorkspace !== null) {
throw new \RuntimeException(sprintf('Failed to create personal workspace "%s" for user with id "%s", because the workspace "%s" is already assigned to the user', $workspaceName->value, $ownerId->value, $existingUserWorkspace->value), 1733754904);
}
$contentRepository = $this->contentRepositoryRegistry->get($contentRepositoryId);
$contentRepository->handle(
CreateWorkspace::create(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,31 @@ public function aPersonalWorkspaceForUserIsCreated(string $username): void
/**
* @Then the personal workspace for user :username is :workspaceName
*/
public function thePersonalWorkspaceForUserIs(string $workspaceName, string $username): void
public function thePersonalWorkspaceForUserIs(string $username, string $workspaceName): void
{
$ownerUserId = $this->userIdForUsername($username);
$actualWorkspace = $this->getObject(WorkspaceService::class)->getPersonalWorkspaceForUser($this->currentContentRepository->id, $ownerUserId);
Assert::assertNotNull($actualWorkspace);
Assert::assertSame($workspaceName, $actualWorkspace->workspaceName->value);
}

/**
* @Then the user :username does not have a personal workspace
*/
public function theUserDoesNotHaveAPersonalWorkspace(string $username): void
{
$ownerUserId = $this->userIdForUsername($username);
try {
$this->getObject(WorkspaceService::class)->getPersonalWorkspaceForUser($this->currentContentRepository->id, $ownerUserId);
} catch (\Throwable $e) {
// todo throw WorkspaceDoesNotExist instead??
Assert::assertInstanceOf(\RuntimeException::class, $e, $e->getMessage());
Assert::assertSame(1733755300, $e->getCode());
return;
}
Assert::fail('Did not throw');
}

/**
* @When the shared workspace :workspaceName is created with the target workspace :targetWorkspace
* @When the shared workspace :workspaceName is created with the target workspace :targetWorkspace and role assignments:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ Feature: Workspace permission related features
Examples:
| user |
| collaborator |
| owner |
# the "owner" user already owns a workspace

Scenario: Creating a workspace without Neos User but READ permissions on live
Given I am not authenticated
Expand All @@ -114,11 +114,11 @@ Feature: Workspace permission related features
Examples:
| user |
| admin |
| owner |
| collaborator |
| uninvolved_editor |
| restricted_editor |
| simple_user |
# the "owner" user already owns a workspace

Scenario Outline: Changing a base workspace without MANAGE permissions or READ permissions on the base workspace
Given I am authenticated as <user>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ Feature: Neos WorkspaceService related features

Scenario: Create a single personal workspace
When the root workspace "some-root-workspace" is created
Then the user "jane.doe" does not have a personal workspace
And the personal workspace "some-user-workspace" is created with the target workspace "some-root-workspace" for user "jane.doe"
Then the personal workspace for user "jane.doe" is "some-user-workspace"
Then the workspace "some-user-workspace" should have the following metadata:
Expand All @@ -109,11 +110,14 @@ Feature: Neos WorkspaceService related features
When a personal workspace for user "test.user" is created
Then the personal workspace for user "test.user" is "test-user-1"

Scenario: For multiple personal workspaces only one workspace is returned
Scenario: A user cannot have multiple personal workspaces
When the root workspace "some-root-workspace" is created
And the personal workspace "b-user-workspace" is created with the target workspace "some-root-workspace" for user "jane.doe"
And the personal workspace "a-user-workspace" is created with the target workspace "some-root-workspace" for user "jane.doe"
And the personal workspace "c-user-workspace" is created with the target workspace "some-root-workspace" for user "jane.doe"
And the personal workspace "b-user-workspace" is created with the target workspace "some-root-workspace" for user "jane.doe"
Then an exception of type "RuntimeException" should be thrown with message:
"""
Failed to create personal workspace "b-user-workspace" for user with id "janedoe", because the workspace "a-user-workspace" is already assigned to the user
"""
Then the personal workspace for user "jane.doe" is "a-user-workspace"

Scenario: Create a single shared workspace
Expand Down

0 comments on commit 31473fb

Please sign in to comment.