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

Fix copy content with descendants not copying sort order (#13464) #13644

Merged
merged 1 commit into from
Jan 13, 2023

Conversation

ustadstar
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes #13464

Description

This PR fixes the issue with copy content not preserving the sort order of the content items.

I have added a SortOrderExists function to the code, to check if the sort order already exists in the tree. This was necessary because otherwise the first parent item which we want to copy would not have a new sort order on the main tree.

Steps to test and reproduce the issue are greatly documented in the issue. I have copied the steps to reproduce down below.

Hi @denhaandrei, thanks for reaching our 💪 I can confirm that this is an issue not only on v10.3.2 but also on previous major Umbraco versions, so the problem has been there for a while... 🤷 The steps I used to reproduce are the ones specified but some additional clarification is needed, so we don't confuse it with a concept in Umbraco called Nested content. 😉

Note ❕ : This is an issue that has been reported in the past (#8033) and there were some attempts to fix it (#8158). Based on my investigation, I would provide some nodes and mark it as up-for-grabs for now 🙂

The issue is about creating a Content node with nested Content nodes (descendants), sorting them and copying the parent Content node - as a result, the sorted order of the copied content nodes is not persisted. In other words:

1. Create a Document type: `"TestSort"`
   1.1. Under _Permissions_ add the same Document type as "_Allowed child node types_" of TestSort

2. Create a content node from TestSort and name it `"Parent item"`

3. Add 3 new items under the Parent item (3 content nodes as direct children)
   ❗ **order is important**:

Nested item B
Nested item A
Nested item C

4. Create 3 new nodes under `"Nested item B"`
   ❗ **order is important**:

Nested Nested Item II
Nested Nested Item I
Nested Nested Item III

6. Sort the following content nodes, like:

Nested item A
Nested item B
Nested item C

8. Sort in Nested item B descendants:

Nested Nested Item I
Nested Nested Item II
Nested Nested Item III

10. Copy the Parent item to the content tree (Relate to original - **uncheck**, Include descendants - **checked**)

We should end up with the following structure:

NOTE: With this PR the structure on the image right should match the copied content structure of the left image.

@github-actions
Copy link

github-actions bot commented Jan 8, 2023

Hi there @ustadstar, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

  • It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
  • The automated tests all pass (see "Checks" tab on this PR)
  • The level of security for this contribution is the same or improved
  • The level of performance for this contribution is the same or improved
  • Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
  • If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
  • 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

@mikecp
Copy link
Contributor

mikecp commented Jan 13, 2023

Hello @ustadstar,

Thanks a lot for providing a solution for this annoying problem 👍
I tested it and it all works as expected, so let's merge this right now 😁

Cheers!

@mikecp mikecp merged commit fd992f6 into umbraco:v11/contrib Jan 13, 2023
@JoseMarcenaro
Copy link
Contributor

Hi @mikecp - is it possible to merge this into the v10 code as well? (I hope this is what the Long Term Support means)

@mikecp
Copy link
Contributor

mikecp commented Feb 3, 2023

Hi @JoseMarcenaro,

Pinging @nul800sebastiaan on this one to decide 😁

Cheers!

@JoseMarcenaro
Copy link
Contributor

Hi @nul800sebastiaan - Allow me to argue my case a little bit 😁

  • this issue is really annoying, it makes copying content almost unusable in a "real world" situation.
  • for technical reasons we cannot migrate to v11 yet (on premises server, .NET 7.0 not available)
  • and it was working fine on latest v8 versions, so it is definitely a step back.

This kind of issue makes it hard for us to sustain the decision of a new major version upgrade, if the v10 upgrade got us into trouble....

Thanks a lot!

@nul800sebastiaan
Copy link
Member

Cherry picked for 10.5 in 0ba1254

@JoseMarcenaro
Copy link
Contributor

Yay! Thanks @nul800sebastiaan !

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.

Umbraco copy content with descendants does not preserve sorting order
4 participants