AvS_FastSimpleImport icon indicating copy to clipboard operation
AvS_FastSimpleImport copied to clipboard

Potential issue: Wrong approach to default category position

Open barbazul opened this issue 9 years ago • 9 comments

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 avatar Feb 02 '16 23:02 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?

barbazul avatar Feb 03 '16 20:02 barbazul

@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.

paales avatar Feb 07 '16 17:02 paales

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.

mbijnsdorp avatar Feb 10 '16 16:02 mbijnsdorp

@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 avatar Feb 12 '16 02:02 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 avatar Jun 13 '17 09:06 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 avatar Jun 21 '17 14:06 barbazul

Should I change the PR base branch to develop?

barbazul avatar Jul 05 '17 22:07 barbazul

@paales

barbazul avatar Nov 08 '17 23:11 barbazul

@avstudnitz

barbazul avatar Dec 31 '17 12:12 barbazul