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.

Sign up for GitHub

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

Jump to bottom

Allow items to be shipped with a qty > 0 and < 1 #35972 #36232

Merged
merged 6 commits into from
Oct 28, 2022
Merged

Allow items to be shipped with a qty > 0 and < 1 #35972 #36232

merged 6 commits into from
Oct 28, 2022

Conversation

engcom-Delta
Copy link
Contributor

Description (*)

Cast the qty to a float, rather than an int. The casted value is checked for <= 0, so anything > 0 and < 1 would be (incorrectly) cast to a 0. If this check fails, the item is not added to the shipment. By casting to a float, the validation is passed correctly and you can now ship items with a qty between 0 and 1.

Related Pull Requests

Fixed Issues (if relevant)

  1. Fixes Unable to ship items with a qty > 0 and < 1 #35971

Manual testing scenarios (*)

  1. ...
  2. ...

Questions or comments

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)
  • README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
  • All automated tests passed successfully (all builds are green)

@m2-assistant
Copy link

m2-assistant bot commented Oct 3, 2022

Hi @engcom-Delta. Thank you for your contribution
Here are some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

  • @magento run all tests - run or re-run all required tests against the PR changes
  • @magento run <test-build(s)> - run or re-run specific test build(s)
    For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

  1. Database Compare
  2. Functional Tests CE
  3. Functional Tests EE,
  4. Functional Tests B2B
  5. Integration Tests
  6. Magento Health Index
  7. Sample Data Tests CE
  8. Sample Data Tests EE
  9. Sample Data Tests B2B
  10. Static Tests
  11. Unit Tests
  12. WebAPI Tests
  13. Semantic Version Checker

You can find more information about the builds here

ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests 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 Pull Requests 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.

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

@m2-community-project m2-community-project bot added the Priority: P2 A defect with this priority could have functionality issues which are not to expectations. label Oct 3, 2022
@engcom-Delta
Copy link
Contributor Author

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@engcom-Delta engcom-Delta self-assigned this Oct 3, 2022
@m2-community-project m2-community-project bot moved this from Pending Review to Review in Progress in High Priority Pull Requests Dashboard Oct 3, 2022
@rogerdz
Copy link
Contributor

rogerdz commented Oct 3, 2022

same #35972

@m2-community-project m2-community-project bot moved this from Review in Progress to Extended Testing (optional) in High Priority Pull Requests Dashboard Oct 3, 2022
@m2-community-project m2-community-project bot moved this from Review in Progress to Extended Testing (optional) in High Priority Pull Requests Dashboard Oct 3, 2022
@m2-community-project m2-community-project bot moved this from Extended Testing (optional) to Pending Review in High Priority Pull Requests Dashboard Oct 3, 2022
@m2-community-project m2-community-project bot moved this from Pending Review to Extended Testing (optional) in High Priority Pull Requests Dashboard Oct 3, 2022
@engcom-Delta
Copy link
Contributor Author

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@engcom-Delta
Copy link
Contributor Author

@magento run Integration Test

@magento-automated-testing
Copy link

Failed to run the builds. Please try to re-run them later.

1 similar comment
@magento-automated-testing
Copy link

Failed to run the builds. Please try to re-run them later.

@engcom-Delta
Copy link
Contributor Author

@magento run Functional Tests CE

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@engcom-Delta
Copy link
Contributor Author

@magento run Integration Tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

5 similar comments
@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@engcom-Delta
Copy link
Contributor Author

Since this is a new PR for PR, In this PR static test failure was fixed and all the checks are passed. Hence moving this PR to Merge in Progress.

Thanks.

@engcom-Delta engcom-Delta moved this from Extended Testing (optional) to Merge in Progress in High Priority Pull Requests Dashboard Oct 12, 2022
@ihor-sviziev
Copy link
Contributor

@engcom-Delta don't you think it should be covered with some automated test?

@engcom-Delta engcom-Delta mentioned this pull request Oct 27, 2022
Closed
5 tasks
@engcom-Delta
Copy link
Contributor Author

Hi @ihor-sviziev @ishakhsuvarov

we have covered and updated the unit testcase in this PR as per the reviewed changes in PR.

Thanks

@engcom-Delta
Copy link
Contributor Author

@magento run all tests

@ishakhsuvarov
Copy link
Contributor

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

@Den4ik
Copy link
Contributor

Den4ik commented Oct 27, 2022

@engcom-Delta That's great that you are added unit tests. But I have a question. ObjectManager is deprecated to use in unit tests and in fact new unit test still use this functional. I see that it's easy to use deprecated functional in same way of other tests, but in fact it's increase amount of legacy code.
Do you think that make sense to replace ObjectManager usage with new command?

@magento-devops-reposync-svc magento-devops-reposync-svc merged commit e83ece5 into magento:2.4-develop Oct 28, 2022
@ishakhsuvarov ishakhsuvarov moved this from Merge in Progress to Recently Merged in High Priority Pull Requests Dashboard Nov 1, 2022
@engcom-Delta
Copy link
Contributor Author

engcom-Delta commented Nov 2, 2022 •
edited
Loading

@Den4ik
Thank you for reviewing. Since this PR was merged, Hence we will update the new PR according to the comment.
Thanks

@engcom-Delta engcom-Delta mentioned this pull request Nov 3, 2022
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@engcom-Delta engcom-Delta

Labels
Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: accept
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unable to ship items with a qty > 0 and < 1
6 participants
@engcom-Delta @rogerdz @ihor-sviziev @ishakhsuvarov @Den4ik @magento-devops-reposync-svc