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.

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

FEATURE: User workspaces #5119

Closed
wants to merge 5 commits into from
Closed

FEATURE: User workspaces #5119

wants to merge 5 commits into from

Conversation

nezaniel
Copy link
Member

@nezaniel nezaniel commented Jun 3, 2024

This introduces the concept of user workspaces on a higher level via the workspace provider

Upgrade instructions

none

Review instructions

none

Checklist

  • Code follows the PSR-12 coding style
  • [] Tests have been created, run and adjusted as needed
  • The PR is created against the 9.0 branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

@nezaniel nezaniel marked this pull request as draft June 3, 2024 12:52
UserId::fromString($this->persistenceManager->getIdentifierByObject($user))
)
);
$this->workspaceProvider->providePrimaryPersonalWorkspaceForUser($siteDetectionResult->contentRepositoryId, $user);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i thought the provide pattern would be used to instantiate the workspace at the first place actually used. This just eagerly instantiates the workspace - which WILL not be the case for the edge case being logged in already and switching sites - so i would probably prefer to remove the functionality from this service?

@mhsdesign
Copy link
Member

I think this can be closed with the introduction of #5146

@mhsdesign mhsdesign closed this Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants