Fixed grid multiselect issue #24400 by mahesh-rajawat · Pull Request #26488 · magento/magento2

mahesh-rajawat · 2020-01-22T07:57:06Z

Grid Multi-Select Column Issue Fixed in this PR

Description (*)

Fixed issues related to multi-select column.

Fixed Issues (if relevant)

UI Component grid indicates a wrong number of selections when removing keyword search #24348

Manual testing scenarios (*)

All the steps verified that given in the issue Issues Found on Grid Multi-Select Column #24400
...

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 · 2020-01-22T07:57:09Z

Hi @maheshWebkul721. 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

For more details, please, review the Magento Contributor Guide documentation.

novikor · 2020-01-23T14:03:08Z

@magento run Static Tests

novikor

Hi, @maheshWebkul721 . Thank you for your contribution.
Please fix the static tests.
Currently, build error is not available, however, I have the problem description:

Stacktrace
disallowTrailingWhitespace: Illegal trailing whitespace at app/code/Magento/Ui/view/base/web/js/grid/columns/multiselect.js :
   238 |            var total = this.getIds().length,
   239 |                selected = this.getPageSelections().length;
   240 |            
--------^
   241 |            if (this.isPageSelected() && selected !== total) {
   242 |                return this.selectPage();

slavvka · 2020-01-23T19:05:27Z

Guys, thank you for the identification of the issue. We are working on fixing that.

slavvka · 2020-01-23T20:11:36Z

Full issue log:

disallowTrailingWhitespace: Illegal trailing whitespace at app/code/Magento/Ui/view/base/web/js/grid/columns/multiselect.js :
   238 |            var total = this.getIds().length,
   239 |                selected = this.getPageSelections().length;
   240 |            
--------^
   241 |            if (this.isPageSelected() &amp;&amp; selected !== total) {
   242 |                return this.selectPage();


requirePaddingNewlinesBeforeKeywords: Keyword `return` should have an empty line above it at app/code/Magento/Ui/view/base/web/js/grid/columns/multiselect.js :
   245 |            if (this.indetermine() &amp;&amp; selected === total) {
   246 |                return this.selectPage();
   247 |            }
---------------------^
   248 |            return this.isPageSelected() ? this.deselectPage() : this.selectPage();
   249 |        },


disallowTrailingWhitespace: Illegal trailing whitespace at app/code/Magento/Ui/view/base/web/js/grid/columns/multiselect.js :
   461 |            this.allSelected(allSelected);
   462 |            this.indetermine(totalSelected &amp;&amp; !allSelected);
   463 |            
--------^
   464 |            return this;
   465 |        },

slavvka · 2020-01-27T15:58:08Z

hey @maheshWebkul721 @novikor We have fixed the issue with static tests https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/26488/a80d343adf054ec702e7edda682c8229/Statics/allure-report-b2b/index.html#suites/3a29d43e5c1686787ff8c2a77e590abb/678be20ee8bc32a5/

shikhamis11 · 2020-02-02T05:06:31Z

Hi
I have also provided solution for above issue in my pull request #26626
Please have a look

…o fixed-24400

no tests coverage

magento-engcom-team · 2020-02-20T16:47:10Z

Hi @novikor, thank you for the review.
ENGCOM-6944 has been created to process this Pull Request
✳️ @novikor, could you please add one of the following labels to the Pull Request?

Label	Description
`Auto-Tests: Covered`	All changes in Pull Request is covered by auto-tests
`Auto-Tests: Not Covered`	Changes in Pull Request requires coverage by auto-tests
`Auto-Tests: Not Required`	Changes in Pull Request does not require coverage by auto-tests

novikor · 2020-02-20T16:48:54Z

Hi, @maheshWebkul721
Thank you for your contribution.
The PR changes are ready for approval, but tests coverage is required.
Don't you mind to cover the PR by MFTF?
@engcom-Delta

magento-engcom-team · 2020-08-19T17:03:56Z

Hi @sidolov, thank you for the review.
ENGCOM-6944 has been created to process this Pull Request

engcom-Alfa · 2020-08-20T09:02:16Z

@magento give me test instance

magento-engcom-team · 2020-08-20T09:02:19Z

Hi @engcom-Alfa. Thank you for your request. I'm working on Magento instance for you

magento-engcom-team · 2020-08-20T09:03:48Z

Hi @engcom-Alfa, here is your new Magento instance.
Admin access: https://pr-26488.instances.magento-community.engineering/admin_8e78
Login: 3ad7d9a1 Password: c71d52aedf26
Instance will be terminated in up to 3 hours.

engcom-Alfa

Hi @maheshWebkul721 .

During testing, we faced the issue.

Manual testing scenario:

Login to the backend;
Create 3 customers (first name, second name, third name);
In the 'Search by keyword' box, enter 'first' and search;
In the first column, choose the 'Select All' option;
Notice that one record is selected:

Remove the 'first' keyword search by pressing the 'x' icon to the right of it.
Notice that it now says that one record is selected,

✖️ but visually all records have a checkmark:

Select Actions => Delete;
Notice that it says it will remove 1 record:
Press OK

Actual Result: ✖️ All products have been removed instead of 1.

@maheshWebkul721 Could you take a look?

Thanks!

Pull Request state was updated. Re-review required.

engcom-Hotel · 2020-09-07T15:54:26Z

@magento run all tests

gabrieldagama · 2020-09-08T10:21:50Z

@magento run all test

sivaschenko · 2020-09-08T10:31:29Z

@magento run all tests

gabrieldagama · 2020-09-15T11:15:46Z

Hi @maheshWebkul721, thanks for your contribution. After looking at the issue and the possible solutions we've decided to proceed with this PR #29968 that aims to fix the same issue.

The approach that was chosen was to preserve the same behavior that we have with filters. So when a search tag is removed from filters the previous selection becomes invalid and it is dismissed. As result, we have the same behavior across multiple functionalities on the grid.

I will be closing this PR but feel free to reopen it anytime.

m2-assistant · 2020-09-15T11:15:48Z

Hi @maheshWebkul721, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Fixed grid multiselect issue magento#24400

bcd0ec9

mahesh-rajawat requested a review from DrewML as a code owner January 22, 2020 07:57

mahesh-rajawat added 3 commits January 22, 2020 15:07

update to fix unit test

3cd6d79

fixed static test

74872b8

syntax error fixed

6928ff9

novikor self-assigned this Jan 22, 2020

novikor added 2 commits January 22, 2020 19:06

Merge branch '2.4-develop' into fixed-24400

0614cee

Merge branch '2.4-develop' into fixed-24400

bc3c22e

novikor self-requested a review January 23, 2020 14:31

novikor requested changes Jan 23, 2020

View reviewed changes

ghost added the Progress: needs update label Jan 23, 2020

Merge branch '2.4-develop' into fixed-24400

4ef8ca5

mahesh-rajawat added 4 commits February 20, 2020 16:14

fixed static test

6cde17f

local changes reverted

2fe4517

Merge branch 'fixed-24400' of github.com:maheshWebkul721/magento2 int…

593334c

…o fixed-24400

Fixed some static test issue

5d57321

mahesh-rajawat requested a review from novikor February 20, 2020 13:04

novikor previously approved these changes Feb 20, 2020

View reviewed changes

magento-engcom-team added Release Line: 2.4 Area: Frontend Component: Ui labels Feb 20, 2020

ghost added Progress: review and removed Progress: needs update labels Jun 30, 2020

sidolov added Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. Severity: S1 Affects critical data or functionality and forces users to employ a workaround. labels Aug 17, 2020

sidolov previously approved these changes Aug 19, 2020

View reviewed changes

ghost assigned sidolov Aug 19, 2020

ghost added Progress: ready for testing and removed Progress: review labels Aug 19, 2020

ghost added Progress: testing in progress and removed Progress: ready for testing labels Aug 20, 2020

engcom-Alfa requested changes Aug 20, 2020

View reviewed changes

ghost added Progress: needs update and removed Progress: testing in progress labels Aug 20, 2020

engcom-Hotel self-assigned this Sep 4, 2020

tgamanov and others added 4 commits September 4, 2020 12:47

Merge branch '2.4-develop' into fixed-24400

2b978d5

The issue with select all action has been fixed.

dd72d8d

Merge branch '2.4-develop' into fixed-24400

0e817ee

Selected count has been fixed. Test has been updated.

bdb6d75

Test has been updated.

cad8b46

gabrieldagama closed this Sep 15, 2020

Conversation

mahesh-rajawat commented Jan 22, 2020 • edited by sidolov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

Uh oh!

m2-assistant bot commented Jan 22, 2020

Uh oh!

novikor commented Jan 23, 2020

Uh oh!

novikor left a comment

Choose a reason for hiding this comment

Uh oh!

slavvka commented Jan 23, 2020

Uh oh!

slavvka commented Jan 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slavvka commented Jan 27, 2020

Uh oh!

shikhamis11 commented Feb 2, 2020

Uh oh!

magento-engcom-team commented Feb 20, 2020

Uh oh!

novikor commented Feb 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

magento-engcom-team commented Aug 19, 2020

Uh oh!

engcom-Alfa commented Aug 20, 2020

Uh oh!

magento-engcom-team commented Aug 20, 2020

Uh oh!

magento-engcom-team commented Aug 20, 2020

Uh oh!

engcom-Alfa left a comment

Choose a reason for hiding this comment

Uh oh!

engcom-Hotel commented Sep 7, 2020

Uh oh!

gabrieldagama commented Sep 8, 2020

Uh oh!

sivaschenko commented Sep 8, 2020

Uh oh!

gabrieldagama commented Sep 15, 2020

Uh oh!

m2-assistant bot commented Sep 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

mahesh-rajawat commented Jan 22, 2020 •

edited by sidolov

Loading

slavvka commented Jan 23, 2020 •

edited

Loading

novikor commented Feb 20, 2020 •

edited

Loading