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

Fix fetching quote item by id #10059

Merged
merged 4 commits into from
Jul 6, 2017
Merged

Fix fetching quote item by id #10059

merged 4 commits into from
Jul 6, 2017

Conversation

mladenilic
Copy link
Contributor

@mladenilic mladenilic commented Jun 26, 2017

Description

Quote item without id (not yet saved) can be added to the collection. Even after being saved, it's still not possible to fetch it $this->getItemsCollection()->getItemById($itemId); as it was added before having the id.

Fixed Issues

  1. Cart 860 does not contain item 1204 #9445: Cart 860 does not contain item 1204

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

Fixes #9445

Fix's issue when quote item without id is added to collection
@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Jun 26, 2017

CLA assistant check
All committers have signed the CLA.

@okorshenko okorshenko self-assigned this Jun 26, 2017
@okorshenko okorshenko added this to the June 2017 milestone Jun 26, 2017
@okorshenko
Copy link
Contributor

Hi @mladenilic
Thank you for your contribution. Could you please cover you fix with integration test so that we can analyze the use case?

@okorshenko
Copy link
Contributor

Hi @mladenilic. Could you please cover you fix with integration test so that we can analyze the use case and proceed with PR acceptance? Thank you

@mladenilic
Copy link
Contributor Author

@okorshenko: Sure! Sorry, for not covering the change with the test initially.

I'll make sure I add the integration test later today.

Best,
Mladen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants