Skip to content

Removed extra qty field from bundle configuration #30617

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
Closed
konarshankar07 wants to merge 24 commits into from
Closed

Removed extra qty field from bundle configuration #30617

konarshankar07 wants to merge 24 commits into from

Conversation

@konarshankar07
Copy link
Contributor

@konarshankar07 konarshankar07 commented Oct 23, 2020 •
edited by engcom-Charlie
Loading

Description (*)

This PR will remove the duplicate qty field from the bundle configure modal whilst creating order from the backend

Related Pull Requests

magento/inventory#3254
https://github.com/magento/partners-magento2b2b/pull/513

Fixed Issues (if relevant)

  1. Fixes Configure Product Modal has duplicated quantity field #30306

Manual testing scenarios (*)

  1. Create a bundle product
  2. Create a new order from admin
  3. Add the bundle product to the order
  4. Click "Configure" link of the bundle product in the "Please select products" grid
  5. Configure Product modal opens
  6. Check the Quantity field

Expected Result

image

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)
  • All automated tests passed successfully (all builds are green)

@m2-assistant
Copy link

m2-assistant bot commented Oct 23, 2020

Hi @konarshankar07. Thank you for your contribution
Here is 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

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, 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, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

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

@konarshankar07
Copy link
Contributor Author

konarshankar07 commented Oct 23, 2020

@magento run all tests

@sivaschenko sivaschenko added Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. labels Oct 23, 2020
@Stepa4man Stepa4man self-assigned this Oct 23, 2020
@Stepa4man Stepa4man self-requested a review October 23, 2020 11:07
Stepa4man
Stepa4man requested changes Oct 23, 2020
View reviewed changes
Copy link
Contributor

@Stepa4man Stepa4man left a comment

Choose a reason for hiding this comment

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

Hi @konarshankar07
Thanks for your contribution.

Please take a look at my comment. And please consider if It makes sense to write some tests or adjust existing to cover those changes

...Magento/Bundle/view/adminhtml/templates/product/composite/fieldset/options/type/select.phtml Outdated
Comment on lines 40 to 45
<?php if ($block->isSelected($_selection)) {
echo ' selected="selected"';
} ?>
<?php if (!$_selection->isSaleable() && !$_skipSaleableCheck) {
echo ' disabled="disabled"';
} ?>
Copy link
Contributor

@Stepa4man Stepa4man Oct 23, 2020

Choose a reason for hiding this comment

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

Please use phtml approach to output something

        <?php if ($someCondition):?>
            <?= $someOutut ?>
        <?php endif; ?>

Please fix it in all places.

@konarshankar07
Copy link
Contributor Author

konarshankar07 commented Oct 23, 2020

@magento run Static Tests

@konarshankar07
Copy link
Contributor Author

konarshankar07 commented Oct 23, 2020

@magento run static tests

@konarshankar07
Copy link
Contributor Author

konarshankar07 commented Oct 23, 2020

@magento run Static Tests

@sivaschenko
Copy link
Member

sivaschenko commented Oct 23, 2020

@magento run all tests

@sivaschenko
Copy link
Member

sivaschenko commented Oct 23, 2020

@konarshankar07 thanks for the pull request! Agree with @Stepa4man it would be good to ensure the functionality is covered with an MFTF test

@engcom-Charlie engcom-Charlie self-assigned this Oct 28, 2020
@engcom-Charlie
Copy link
Contributor

engcom-Charlie commented Oct 28, 2020

Hi @konarshankar07 could you please look at failed tests and cover your changes by some kind of autotest?
Thank you.

@engcom-Charlie
Copy link
Contributor

engcom-Charlie commented Oct 30, 2020

Hi @konarshankar07 please look at failed tests and cover your changes by some kind of autotest.
Thank you.

@konarshankar07
Copy link
Contributor Author

konarshankar07 commented Oct 30, 2020

Hello @engcom-Charlie ..
Sorry for the delay.. I will cover the changes with autotest and fix the failed test in this weekend
Thanks

@konarshankar07
Copy link
Contributor Author

konarshankar07 commented Nov 1, 2020

@magento run Functional Tests CE

@engcom-Charlie
Copy link
Contributor

engcom-Charlie commented Nov 2, 2020

Hi @konarshankar07 looks like we still have failed MFTF tests. Could you please fix them?
Thank you.

@engcom-Charlie
Copy link
Contributor

engcom-Charlie commented Nov 6, 2020

Hi @konarshankar07 could you please fix the failed MFTF tests?
Thank you.

@ishakhsuvarov
Copy link
Contributor

ishakhsuvarov commented Nov 10, 2022

@magento run all tests

@magento-automated-testing
Copy link

magento-automated-testing bot commented Nov 10, 2022

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.

@ishakhsuvarov
Copy link
Contributor

ishakhsuvarov commented Nov 10, 2022

Moving to Extended testing to ensure green tests.

@engcom-Echo engcom-Echo self-assigned this Nov 11, 2022
@engcom-Echo engcom-Echo removed their assignment Nov 11, 2022
@engcom-Echo engcom-Echo self-assigned this Nov 11, 2022
@engcom-Echo
Copy link
Contributor

engcom-Echo commented Nov 11, 2022

@magento run all tests

@magento-automated-testing
Copy link

magento-automated-testing bot commented Nov 11, 2022

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-Echo
Copy link
Contributor

engcom-Echo commented Nov 11, 2022

@magento run Integration Tests, Functional Tests EE, Functional Tests CE, Functional Tests B2B

@magento-automated-testing
Copy link

magento-automated-testing bot commented Nov 11, 2022

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-Echo
Copy link
Contributor

engcom-Echo commented Nov 14, 2022

@magento run all tests

@magento-automated-testing
Copy link

magento-automated-testing bot commented Nov 14, 2022

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-Echo
Copy link
Contributor

engcom-Echo commented Nov 14, 2022

@magento run Sample Data Tests EE, Sample Data Tests CE, Sample Data Tests B2B, Integration Tests, Functional Tests EE, Functional Tests CE, Functional Tests B2B

@magento-automated-testing
Copy link

magento-automated-testing bot commented Nov 14, 2022

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-Echo
Copy link
Contributor

engcom-Echo commented Nov 14, 2022 •
edited
Loading

we are closing this PR.

As this is not an issue #30306.
As first quantity field represents the quantity of option only in case of (radio or dropdown option type) only if user defined is checked.
the other quantity represent the quantity of bundle product.
For reference you can see this docs

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

Reviewers

@engcom-Charlie engcom-Charlie engcom-Charlie left review comments

@Stepa4man Stepa4man Stepa4man approved these changes

Assignees

@Stepa4man Stepa4man

@engcom-Bravo engcom-Bravo

@engcom-Charlie engcom-Charlie

@engcom-Echo engcom-Echo

Labels

Area: Frontend Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Component: Bundle Component: Sales Event: MageCONF CD 2020 Priority: P2 A defect with this priority could have functionality issues which are not to expectations. QA: Added to Regression Scope Scenario was analysed and added to Regression Testing Scope Release Line: 2.4 Severity: S2 Major restrictions or short-term circumventions are required until a fix is available.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Configure Product Modal has duplicated quantity field

8 participants

@konarshankar07 @sivaschenko @engcom-Charlie @magento-engcom-team @engcom-Bravo @ishakhsuvarov @engcom-Echo @Stepa4man