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

Jump to bottom

FEATURE: Edit, delete and switch workspaces #4186

Merged
merged 8 commits into from
Apr 16, 2023
Merged

FEATURE: Edit, delete and switch workspaces #4186

merged 8 commits into from
Apr 16, 2023

Conversation

dlubitz
Copy link
Contributor

@dlubitz dlubitz commented Apr 12, 2023 •
edited
Loading

Adds following features using the new CR:

  • edit workspaces
  • delete workspaces
  • switching workspaces

Fixes #4041

@dlubitz dlubitz added the 9.0 label Apr 12, 2023
@dlubitz dlubitz self-assigned this Apr 12, 2023
@dlubitz dlubitz changed the title WIP: FEATURE: Handle updates to workspace and deletion FEATURE: Handle updates to workspace and deletion Apr 12, 2023
@dlubitz dlubitz changed the title FEATURE: Handle updates to workspace and deletion FEATURE: Edit and delete workspaces Apr 12, 2023
@dlubitz dlubitz requested a review from skurfuerst April 12, 2023 21:22
@dlubitz dlubitz requested review from bwaidelich, skurfuerst and ahaeslich and removed request for skurfuerst and bwaidelich April 12, 2023 21:22
@dlubitz dlubitz marked this pull request as ready for review April 12, 2023 21:23
@dlubitz dlubitz changed the title FEATURE: Edit and delete workspaces FEATURE: Edit, delete and switch workspaces Apr 14, 2023
@dlubitz dlubitz mentioned this pull request Apr 14, 2023
Merged
@skurfuerst
Copy link
Member

Awesome!! I'll review this in the morning (as it is a rather big change ;) )

@dlubitz
Copy link
Contributor Author

dlubitz commented Apr 15, 2023 via email •
edited
Loading

@skurfuerst
Copy link
Member

@dlubitz There would be two ways to do it I think:

  1. ask the projection (this would be a soft constraint check then, and IMHO there's nothing wrong with them).  -> Ah, but the projection is inside the wrong package. Indeed.

  2. Figure out whether the content stream belonging to the workspace has events/commands on it - by using this method (or s.th. like it) https://github.com/neos/neos-development-collection/blob/9.0/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php#L423

All the best,
Sebastian

skurfuerst
skurfuerst reviewed Apr 15, 2023
View reviewed changes
Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php Outdated
// TODO Check workspace is empty before forking
$changedNodes = 0;
if ($changedNodes > 0) {
throw new WorkspaceIsNotEmptyException('The user workspace needs to be empty before forking.', 1681455989);
Copy link
Member

Choose a reason for hiding this comment

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

needs to be empty before switching the base workspace (naming)

skurfuerst
skurfuerst reviewed Apr 15, 2023
View reviewed changes
Copy link
Member

@skurfuerst skurfuerst left a comment

Choose a reason for hiding this comment

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

@dlubitz really cool change :) Just found a few nitpicks, I think this is really close to merging! Thanks!!

@dlubitz
Copy link
Contributor Author

dlubitz commented Apr 16, 2023

Thanks for for your rewiew @skurfuerst, I've implemented all changes will have a look into the last open task next days.

@dlubitz
Copy link
Contributor Author

dlubitz commented Apr 16, 2023

Done. Also added the check for empty workspace on base workspace change.

@dlubitz dlubitz requested a review from skurfuerst April 16, 2023 17:43
@skurfuerst
Copy link
Member

@dlubitz great work ❤️ ❤️ ❤️

@skurfuerst skurfuerst merged commit 8f496c4 into neos:9.0 Apr 16, 2023
@dlubitz dlubitz deleted the feature-workspace-changes branch April 16, 2023 21:27
This was referenced Sep 11, 2023
Closed
Merged
@mhsdesign mhsdesign mentioned this pull request Dec 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skurfuerst skurfuerst skurfuerst approved these changes

@ahaeslich ahaeslich Awaiting requested review from ahaeslich

@bwaidelich bwaidelich Awaiting requested review from bwaidelich

Assignees

@dlubitz dlubitz

Labels
9.0 Feature
Projects
Neos 9.0 Release Board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dlubitz @skurfuerst