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

BUGFIX: Throw PartialWorkspaceRebaseFailed if workspace was up to date and publish didnt work #5370

Conversation

mhsdesign
Copy link
Member

@mhsdesign mhsdesign commented Nov 15, 2024

Resolves partially #5364

PartialWorkspaceRebaseFailed exception

Thrown if the partial publish/discard cannot work because the events cannot be reordered as filtered.

This can happen for cases like attempting to publish a removal first and wanting as remaining change
a node move out of the removed descendants or publishing a node variant creation before the node is created.

We cannot reliably detect these cases in advance but in case the workspace is up-to-date its most likely such
an ordering conflict.

To solve the problem the partial operation should be retried with a different filter or a full publish/discard is required.

If the workspace is outdated we cannot know for sure but suspect first that the conflict arose due to changes
in the base workspace, thus we throw WorkspaceRebaseFailed instead.
A forced rebase then might not solve the problem if It's because the order of events cannot be changed.
But attempting a second partial publish/discard (with up-to-date workspace) this exception will be thrown and can be reacted upon.

Upgrade instructions

Review instructions

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained 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

@kitsunet
Copy link
Member

Shouldn't we merge this for now as a safety measure, not much we can do right now....

…information what exactly went wrong

The Neos Ui should be able to catch this and tell the user to publish something else first or publish all.
@mhsdesign mhsdesign marked this pull request as ready for review December 1, 2024 09:33
@mhsdesign mhsdesign changed the title BUGFIX: #5364 throw better exceptions if workspace was up to date and publish didnt work BUGFIX: #5364 throw PartialWorkspaceRebaseFailed if workspace was up to date and publish didnt work Dec 1, 2024
@mhsdesign mhsdesign changed the title BUGFIX: #5364 throw PartialWorkspaceRebaseFailed if workspace was up to date and publish didnt work BUGFIX: Throw PartialWorkspaceRebaseFailed if workspace was up to date and publish didnt work Dec 1, 2024
@mhsdesign
Copy link
Member Author

Okay i introduced a proper PartialWorkspaceRebaseFailed exception for this reorder case, added some documentation and a test for the issue with the move node and delete;)

now id say yes merge! The neos ui will still need some love making this a human readable error and also to offer a DiscardAll or PublishAll button (or maybe at some point publishAll parent nodes too ^^)

@mhsdesign mhsdesign requested review from kitsunet and dlubitz December 1, 2024 10:05
@kitsunet
Copy link
Member

kitsunet commented Dec 1, 2024

Mmmm, the test error seems legit, I'll have a look

@kitsunet kitsunet merged commit 7d648f2 into neos:9.0 Dec 1, 2024
9 checks passed
@mhsdesign mhsdesign deleted the bugfix/5364-throw-better-exceptions-if-workspace-was-up-to-date-and-publish-didnt-work branch December 1, 2024 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants