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

Tier Price Storage Interface Delete Bug doesn't delete all Tier Prices #37949

Open
1 of 5 tasks
MarcProvi opened this issue Sep 1, 2023 · 12 comments
Open
1 of 5 tasks
Labels
Area: Framework Component: Catalog Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: ready for dev Reported on 2.4.x Indicates original Magento version for the Issue report. Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it

Comments

@MarcProvi
Copy link

Preconditions and environment

  • Magento Open Source 2.4.X
  1. Clean Installation of Magento 2.4.X, I used 2.4.6-p2 but all 2.4.X are affected.
  2. Magento with multiple websites.
  3. Product has assigned multiple websites.
  4. Magento Sample Data installed and Tier prices are added to the products with identical information except the website that is affected.

Steps to reproduce

  1. Go to the admin edit page and edit a product information.
  2. Set product tier prices with identical information but different affected websites.
  3. Delete ALL product tier prices with TierPriceStorageInterface, to do this get all tier prices with the TierPriceStorageInterface and use the retrieved prices to delete them.

Tier Prices should look like this:

image

Expected result

All Tier prices are deleted.

Actual result

Only the first tier price that match the conditions is deleted, because it doesn't take into account the website of the tier price.

Additional information

The problem is found in \Magento\Catalog\Model\Product\Price\TierPriceStorage.php where the function delete doesn't retrieve correctly the affected IDs to delete. The function retrievePriceId (Line 267) is the culprit of the bug with the current code:
` /**
* Look through provided price in list of existing prices and retrieve it's Id.
*
* @param array $price
* @param array $existingPrices
* @return int|null
*/
private function retrievePriceId(array $price, array $existingPrices): ?int
{
$linkField = $this->tierPricePersistence->getEntityLinkField();

    foreach ($existingPrices as $existingPrice) {
        if ($existingPrice['all_groups'] == $price['all_groups']
            && $existingPrice['customer_group_id'] == $price['customer_group_id']
            && $existingPrice['qty'] == $price['qty']
            && $this->isCorrectPriceValue($existingPrice, $price)
            && $existingPrice[$linkField] == $price[$linkField]
        ) {
            return (int) $existingPrice['value_id'];
        }
    }

    return null;
}`

But the correct way of retrieving the price Id should be:
` /**
* Look through provided price in list of existing prices and retrieve it's Id.
*
* @param array $price
* @param array $existingPrices
* @return int|null
*/
private function retrievePriceId(array $price, array $existingPrices): ?int
{
$linkField = $this->tierPricePersistence->getEntityLinkField();

    foreach ($existingPrices as $existingPrice) {
        if ($existingPrice['all_groups'] == $price['all_groups']
            && $existingPrice['customer_group_id'] == $price['customer_group_id']
            && $existingPrice['qty'] == $price['qty']
            && $this->isCorrectPriceValue($existingPrice, $price)
            && $existingPrice[$linkField] == $price[$linkField]
            && $existingPrice['website_id'] == $price['website_id']
        ) {
            return (int) $existingPrice['value_id'];
        }
    }

    return null;
}`

With this fix the retrieve price ID takes into account the website the tier price is created and makes a unique identifier of the tier price with all the previous information.

Release note

No response

Triage and priority

  • Severity: S0 - Affects critical data or functionality and leaves users without workaround.
  • Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
  • Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
  • Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
  • Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.
@m2-assistant
Copy link

m2-assistant bot commented Sep 1, 2023

Hi @MarcProvi. Thank you for your report.
To speed up processing of this issue, make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, Add a comment to the issue:


Join Magento Community Engineering Slack and ask your questions in #github channel.
⚠️ According to the Magento Contribution requirements, all issues must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.
🕙 You can find the schedule on the Magento Community Calendar page.
📞 The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

@m2-assistant
Copy link

m2-assistant bot commented Sep 1, 2023

Hi @engcom-Bravo. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: 👇

  • 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
  • 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue.
  • 3. Add Area: XXXXX label to the ticket, indicating the functional areas it may be related to.
  • 4. Verify that the issue is reproducible on 2.4-develop branch
    Details- Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
    - If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
    - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
  • 5. Add label Issue: Confirmed once verification is complete.
  • 6. Make sure that automatic system confirms that report has been added to the backlog.

@engcom-Bravo engcom-Bravo added the Reported on 2.4.x Indicates original Magento version for the Issue report. label Sep 1, 2023
@okolesnyk
Copy link
Member

@magento give me 2.4-develop instance

@magento-deployment-service
Copy link

Hi @okolesnyk. Thank you for your request. I'm working on Magento instance for you.

@magento-deployment-service
Copy link

@engcom-Bravo engcom-Bravo removed their assignment Sep 4, 2023
@engcom-Bravo engcom-Bravo added the Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it label Sep 4, 2023
@engcom-November engcom-November self-assigned this Sep 29, 2023
@m2-assistant
Copy link

m2-assistant bot commented Sep 29, 2023

Hi @engcom-November. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: 👇

  • 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
  • 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue.
  • 3. Add Area: XXXXX label to the ticket, indicating the functional areas it may be related to.
  • 4. Verify that the issue is reproducible on 2.4-develop branch
    Details- Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
    - If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
    - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
  • 5. Add label Issue: Confirmed once verification is complete.
  • 6. Make sure that automatic system confirms that report has been added to the backlog.

@engcom-November
Copy link
Contributor

Hello @MarcProvi,

Thank you for the report and collaboration!

Can you please elaborate the 4th step in Preconditions, and what exactly you mean by the affected website.
This will help us to reproduce the issue more effectively.
It would be great if you can provide sample code, or screencast for the reference.

Thank you.

@engcom-November engcom-November added the Issue: needs update Additional information is require, waiting for response label Sep 29, 2023
@MarcProvi
Copy link
Author

With the 4th Step of preconditions I mean that two tier prices exist with the same information but with different websites, as the image of steps to reproduce shows.

So for example in the tier prices will be:
Tier Price 1:

  • Website: French website
  • Group: General
  • Quantity: 1
  • Price Type: Fixed
  • Price: 299,99

Tier Price 2:

  • Website: Spanish website
  • Group: General
  • Quantity: 1
  • Price Type: Fixed
  • Price: 299,99

The only data difference between the two tier prices is the website they are assigned to, the other data is exactly the same.

The problem is that the delete tier price storage function checks all the fields despite the website ID before deleting the tier price, so if two tier prices match all the other fields but with different website, the tier price storage function will retrieve only the first result of the tier prices and not both IDs when trying to delete both tier prices.

An example of use would be that you want to delete all tier prices of a specified product. When retrieving all tier prices of the product and using the tier price storage interface to delete all tier prices, all tier prices should be deleted, but in reality only the first tier price is deleted and the other not.

@engcom-November
Copy link
Contributor

Hello @MarcProvi,

Thank you for the quick response!

We followed your inputs and the issue seems to be reproducible on 2.4-develop.
We created a custom module to delete the tier price after opening the specific route.
Please take a look at the screenshots:

Here we have created same tier prices for two different website.
image

After using the price storage interface to delete the tier prices we got below result.
image
We can see here that only one tier price has been deleted, hence confirming the issue.

Please find the custom module used to reproduce the issue.
TierVendor.zip

Thank you.

@engcom-November engcom-November added Component: Catalog Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Area: Framework labels Oct 16, 2023
@m2-community-project m2-community-project bot removed the Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed label Oct 16, 2023
@github-jira-sync-bot
Copy link

✅ Jira issue https://jira.corp.adobe.com/browse/AC-9738 is successfully created for this GitHub issue.

@m2-community-project m2-community-project bot added Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed and removed Issue: needs update Additional information is require, waiting for response labels Oct 16, 2023
@m2-assistant
Copy link

m2-assistant bot commented Oct 16, 2023

✅ Confirmed by @engcom-November. Thank you for verifying the issue.
Issue Available: @engcom-November, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

@github-jira-sync-bot
Copy link

❌ You don't have permission to export this issue.

@engcom-Hotel engcom-Hotel added the Priority: P2 A defect with this priority could have functionality issues which are not to expectations. label Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Framework Component: Catalog Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: ready for dev Reported on 2.4.x Indicates original Magento version for the Issue report. Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it
Projects
Status: Ready for Development
Development

No branches or pull requests

6 participants