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

[Catalog] Fix #2948, fallback to the last known valid product positions #3368

Conversation

vahonc
Copy link
Collaborator

@vahonc vahonc commented Sep 4, 2024

No description provided.

@rbayet rbayet linked an issue Sep 7, 2024 that may be closed by this pull request
@rbayet rbayet self-requested a review September 16, 2024 08:55
@rbayet rbayet assigned rbayet and unassigned romainruaud Sep 16, 2024
*/
private function unserializeProductPositions(\Magento\Catalog\Model\Category $category)
{
$productPositions = $category->getSortedProducts() ? $category->getSortedProducts() : [];
// Get product positions from the category.
$productPositions = $category->getSortedProducts();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this line was changed but it does not seem to affect the behaviour of the feature or fix so far, even with multi-store and per-store view positions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code now directly fetches product positions from the category using $category->getSortedProducts(). It will return either the current list of sorted products or null/an empty array if none are set. Therefore, there's no need to use a ternary operator to check and assign an empty array. Then if the data returned by $category->getSortedProducts() is not a valid JSON string (e.g., it's malformed or truncated), the jsonDecode() function will fail to parse it, the fallback is to use getProductPositionsByCategory($category). The warning message is displayed to notify the admin about the fallback, providing a clear indication that something went wrong.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point is about he ternary operator removal.

It will return either the current list of sorted products or null/an empty array if none are set.

Before, it would either return the currently submitted list of sorted product (in a not 100% determined format) or an empty array.
The only thing preventing to do a $category->setSortedProducts($productPositions); is the initial check if ($category->getId() && $category->getSortedProducts()) {.

@rbayet rbayet assigned vahonc and unassigned rbayet Sep 16, 2024
@rbayet
Copy link
Collaborator

rbayet commented Sep 16, 2024

Replaced by #3386

@rbayet rbayet closed this Sep 16, 2024
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.

Manual sort gets set back to automatic sort
3 participants