Fix "jumping" taxons while moving up or down

[jakubtobiasz]

Q	A
Branch?	1.12
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Related tickets	fixes #9909, based on #11905
License	MIT

Basing on #11905 I removed the "feature" part, to merge the "bug-fixing" part into 1.12. We'll consider later adding "Move top" and "Move end" features, but probably it'll be considered in terms of 2.0 with the new Admin Panel.

Thanks @igormukhingmailcom for your work! I'm happy we're finally going to merge your commits :).

Previously:

Now:

[github-actions]

Bunnyshell Preview Environment deleted

Available commands:

• /bns:deploy to redeploy the environment

[SzymonFilipek]

Changes passed QA check

[igormukhingmailcom]

Thanks for the fixes, @jakubtobiasz :)

[ehibes]

Hi guys,
Why not use nested set behavior for taxons position ? It’s already implemented on database... otherwise I don't see what use it would be.
#7585 this issue is about it too.

[jakubtobiasz]

Hi @ehibes 👋🏼,
thanks for sharing your thoughts. This PR aims to fix the existing behavior, the issue you share might be considered in a longer term (e.g. Sylius 2.0 or 3.0 as it might require analysis + refactoring and changes in the behavior).

[jakubtobiasz]

Thanks for leaving the review @igormukhingmailcom 😅

[diimpp]

Is it necessary in presence of getOneOrNullResult? Which will throw exception like so

        if (count($result) > 1) {
            throw new NonUniqueResultException();
        }

[jakubtobiasz]

I wanted to eliminate a need to handle the exception.

[diimpp]

Same question

[jakubtobiasz]

I wanted to eliminate a need to handle the exception.

[diimpp]

New repository methods are unused anywhere in the codebase

[igormukhingmailcom]

Is this works?

$taxonToBeMoved->setPosition($taxonToBeMoved->getPosition() - 1);

It just doing the same -1 as we had before (when things did not work), but on backend side rather than on front-end side - that is all the difference. Did someone tested this by hands?

[jakubtobiasz]

@igormukhingmailcom

There's an if statement preventing moving the taxon to -1 positions. Behat tests are passing that way, and we tested it yesterday.

[GSadee]

Suggested change

	And the first taxon on the list should be "T-Shirts"
	And the first taxon on the list should still be "T-Shirts"

This would describe the behaviour even better, the same in the below scenario

[GSadee]

Thank you, Jacob! 🥇