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: Add dimensions to Workspace module #3986

Merged

Conversation

pKallert
Copy link
Contributor

@pKallert pKallert commented Dec 22, 2022

resolves: #3470

This adds Dimension handling to the current workspaces module.

Currently the Module only displays changes in different languages as one change. This is because the node path is the same in different languages after they are copied.

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

@pKallert pKallert changed the title Feature: Add dimensions to Workspace module Bugfix: Add dimensions to Workspace module Dec 22, 2022
@crydotsnake crydotsnake changed the title Bugfix: Add dimensions to Workspace module FEATURE: Add dimensions to Workspace module Dec 23, 2022
@crydotsnake crydotsnake changed the title FEATURE: Add dimensions to Workspace module BUGFIX: Add dimensions to Workspace module Dec 23, 2022
Copy link
Member

@markusguenther markusguenther left a comment

Choose a reason for hiding this comment

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

It works fine, and we have just nitpicks in the code.

@markusguenther
Copy link
Member

Before:
Screenshot 2023-03-29 at 14 08 56

After:
Screenshot 2023-03-29 at 14 08 27

@markusguenther
Copy link
Member

Would be nice if someone can have a second look @kdambekalns @mhsdesign @kitsunet 🙏

@mhsdesign mhsdesign self-requested a review March 29, 2023 12:45
@markusguenther
Copy link
Member

No one else available for a review @kitsunet @kdambekalns or @Sebobo
It would be nice to have a second review, thanks in advance. :)

@kitsunet
Copy link
Member

kitsunet commented Apr 19, 2023

I have actually done something like this before in a customer project, I have to compare and remember the edge cases I encountered, so I need a moment, I am pretty sure there were some problems with this (eg. if you create a document in a dimension that also has another dimension as fallback, and then underneath two documents for both dimensions, you shouldn't publish the fallbck sub document as it would miss a parent in that case, stuff like that. this case becomes even worse for content and fallbacks).

@markusguenther
Copy link
Member

markusguenther commented Aug 17, 2023

Certainly, @kitsunet ! It's great to hear that you have experience with a similar scenario in a customer project. Your insights into potential issues are valuable. To proceed, could you please provide more details about the specific problems you encountered during that project? Understanding the edge cases and challenges you faced will help us address those issues effectively and come up with solutions to ensure a smooth implementation. Please don't hesitate to share any specific scenarios, complications, or concerns you remember encountering. This will enable us to discuss and brainstorm potential solutions to tackle those issues and ensure a successful outcome for your current project.

@markusguenther markusguenther force-pushed the feature/add-workspace-dimension-handling branch from e7beadc to 1b96bef Compare August 17, 2023 12:40
@markusguenther
Copy link
Member

Rebased changes so that the PR is mergeable until we find a solution for the discussion :)

@mhsdesign
Copy link
Member

How do we want to proceed here? This will definitely be not easy to upmerge so we need to reimplement this for neos 9 as well.

@markusguenther
Copy link
Member

In the Neos 9.0 weekly we talked today about the PR and @kitsunet mentioned that he is fine with merging it.

@pKallert pKallert requested a review from kitsunet January 9, 2024 15:03
@markusguenther markusguenther merged commit d50ea72 into neos:7.3 Jan 17, 2024
8 checks passed
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.

6 participants