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

Distinguish between default zero and intentional zero sort order for new documents #17517

Open
wants to merge 2 commits into
base: v15/dev
Choose a base branch
from

Conversation

Migaroez
Copy link
Contributor

@Migaroez Migaroez commented Nov 12, 2024

Fixes #17273

Description

The current PersistNewItem logic in DocumentRepository checks whether the sort order already exits in the database and changes the sortOrder if so, else persists the sortorder on the entity. This was added in #13644 specifically for documents to support maintaining the sortorder on copy with descendants which is an action that is only supported for documents.

Since the default value of an int is 0 and the lowest sortOrder we support is 0, we can not tell the difference between

  • the value not being set => 0
  • the value being set implicitly (for example when copying with descendants) => 0

This PR proposes to

  • Change the default initialization value of the backing field of the TreeEntityBase class to -1
  • Use this value to check default vs intention

Considerations

  • All repositories that persist new values of derived classes of TreeEntityBase implicetly update the sortOrder value, so a -1 will not be persisted
  • By setting the initialization value of the backing field, we do not have to fight with DetectChanges mechanics and when a new instance is being created from persisted data, this value is guaranteed to be overwritten

Testing

  • Make sure the linked issue is fixed
  • the fix made by the linked PR is still active
  • no side effects with any of the TreeEntityBase derived classes exist.

@Migaroez Migaroez changed the base branch from contrib to v15/dev November 12, 2024 16:02
@Migaroez Migaroez changed the title V15/fix/new document sort order Distinguish between default zero and intentional zero sort order for new documents Nov 12, 2024
@nikolajlauridsen
Copy link
Contributor

Just so this doesn't get lost in the ether.

I think we should use the change tracking system to determine if SortOrder was changed rather than rely on -1 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Moving item to bin doesn't update its siblings sort order
2 participants