Potential issue: Wrong approach to default category position

[barbazul]

In https://github.com/avstudnitz/AvS_FastSimpleImport/blob/master/src/app/code/community/AvS/FastSimpleImport/Model/Import/Entity/Category.php#L477 you are setting the category position to 10000 when it is not specified.

I am currently tracking an issue that apparently relates to this.

The symptoms are, after an import whenever I rearrange a category within its parent the new position gets set to 10001, sending it to the bottom.

Afterwards whenever I rearrange another category in the same parent, the first category gets reverted back to 10000 and the new category gets assigned to 10001.

This is because of the logic inside Mage_Catalog_Model_Resource_Category::_processPositions

I am unsure about how best to approach this issue. Suggestions?

[barbazul]

We have successfully confirmed this issue for different scenarios (changing parent, rearranging within the same parent and deleting categories with siblings).

I am still not sure what the bes approach would be. Perhaps preload the max(position) for each parent_id and then autoincrement it for each child that gets added?

[paales]

@barbazul Your suggested solution might solve the issue. Please create a PR to solve the issue :) You can create the PR on the develop branch.

[mbijnsdorp]

I ran into this as well and started thinking about preloading the max position. However that might prove difficult as you'd have to recalculate it if a category is moved to a new parent during the import.
Why is the position set to 10000 in the first place? If I remove that part, the category is imported just fine at position 0.

[barbazul]

@mbijnsdorp I am not sure about leaving it at 0 would fix the issue. I think trouble beings when more than one category has a repeated position value, be it 0, 100000 or any other.

@paales I have some spare time right now, I'll work on a PR and try to commit later tonight

[barbazul]

I created a new PR request with a more up to date version of @mbijnsdorp solution.

Please consider it for merging as this issue has been open for over a year now

[barbazul]

I would love to have some feedback on this issue as I find myself with some spare time to work some more on this if needed

[barbazul]

Should I change the PR base branch to develop?

[barbazul]

@paales

[barbazul]

@avstudnitz