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

[#25963] Fixed grids export: option labels are taken from grid filters and columns now. #26523

Merged
merged 13 commits into from
Feb 21, 2020
Merged

[#25963] Fixed grids export: option labels are taken from grid filters and columns now. #26523

merged 13 commits into from
Feb 21, 2020

Conversation

novikor
Copy link
Contributor

@novikor novikor commented Jan 24, 2020 •
edited
Loading

Grid Export rendered data is not reflecting in the exported File, Displayed ID instead of Rendered Label #25963.
Fixed grids export: option labels are taken from grid filters and columns now.

Description (*)

A universal solution has been provided: now all the exported grids (Magento's and custom as well) should have option labels instead if option values for all the columns with type Select of Multiselect.

Fixed Issues (if relevant)

  1. Grid Export rendered data is not reflecting in the exported File, Displayed ID instead of Rendered Label #25963: Grid Export rendered data is not reflecting in the exported File, Displayed ID instead of Rendered Label

Manual testing scenarios (*)

See #25963

Questions or comments

A bunch of classes that acts as workaround for the issue should be deprecated/removed:
\Magento\Customer\Ui\Component\DataProvider\Document
\Magento\Sales\Ui\Component\DataProvider\Document

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)

@novikor
Grid Export rendered data is not reflecting in the exported File, Dis…
b43980b 
…played ID instead of Rendered Label magento#25963.

Fixed grids export: option labels are taken from grid filters and columns now.
@novikor novikor requested a review from melnikovi as a code owner January 24, 2020 13:12
@m2-assistant
Copy link

m2-assistant bot commented Jan 24, 2020

Hi @novikor. 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
Grid Export rendered data is not reflecting in the exported File, Dis…
93793d8 
…played ID instead of Rendered Label magento#25963.

Implemented strict types.
@novikor novikor requested a review from swnsma January 24, 2020 13:37
@novikor novikor mentioned this pull request Jan 24, 2020
Closed
4 tasks
@novikor
Grid Export rendered data is not reflecting in the exported File, Dis…
c517f5f 
…played ID instead of Rendered Label magento#25963.

Fixed case when options are configured via XML directly instead of providing a class.
nuzil
nuzil requested changes Jan 27, 2020
View reviewed changes
Copy link
Contributor

@nuzil nuzil left a comment

Choose a reason for hiding this comment

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

@novikor
Can you please check failed tests?

@ghost ghost assigned nuzil Jan 27, 2020
@novikor
Copy link
Contributor Author

novikor commented Jan 28, 2020

Thank you for your review, @nuzil.
Will do.

@novikor novikor removed the request for review from swnsma January 30, 2020 11:53
@novikor novikor force-pushed the 25963-grid-export-labels branch from 1776b7a to c517f5f Compare January 30, 2020 16:48
@novikor
Grid Export rendered data is not reflecting in the exported File, Dis…
9bb416f 
…played ID instead of Rendered Label magento#25963.

Fixed Unit tests.
@novikor
Grid Export rendered data is not reflecting in the exported File, Dis…
cd24e41 
…played ID instead of Rendered Label magento#25963.

Unit tests code reformatting.
@novikor
Grid Export rendered data is not reflecting in the exported File, Dis…
6b434ba 
…played ID instead of Rendered Label magento#25963.

Implemented Unit tests coverage.
@novikor novikor force-pushed the 25963-grid-export-labels branch from e0ee531 to 8380148 Compare January 31, 2020 10:25
@novikor
Copy link
Contributor Author

novikor commented Jan 31, 2020 •
edited
Loading

Checked: \Magento\Customer\Ui\Component\DataProvider\Document can not be removed or deprecated.
\Magento\Sales\Ui\Component\DataProvider\Document was safely deleted.

@novikor novikor requested a review from nuzil January 31, 2020 10:27
@novikor
Grid Export rendered data is not reflecting in the exported File, Dis…
b117b0c 
…played ID instead of Rendered Label magento#25963.

Removed deprecated class.
@novikor novikor force-pushed the 25963-grid-export-labels branch from b16df93 to b117b0c Compare January 31, 2020 11:17
@novikor
Grid Export rendered data is not reflecting in the exported File, Dis…
4a78ca5 
…played ID instead of Rendered Label magento#25963.

Removed deprecated class.
@novikor novikor force-pushed the 25963-grid-export-labels branch from 7f1e960 to 4a78ca5 Compare January 31, 2020 12:54
@novikor
Copy link
Contributor Author

novikor commented Jan 31, 2020

@nuzil - done, please review.
The failed functional test is reproduced on 2.4-develop: #26607

@novikor
Copy link
Contributor Author

novikor commented Feb 3, 2020

@magento run all tests

@novikor
Copy link
Contributor Author

novikor commented Feb 6, 2020 •
edited
Loading

@magento run Functional Tests CE

@novikor
Copy link
Contributor Author

novikor commented Feb 6, 2020

@magento run Functional Tests EE

@novikor
Copy link
Contributor Author

novikor commented Feb 11, 2020

It seems that the PR will never be approved because of flaky tests

@nuzil nuzil added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Feb 14, 2020
@magento-engcom-team
Copy link
Contributor

Hi @nuzil, thank you for the review.
ENGCOM-6901 has been created to process this Pull Request
✳️ @nuzil, 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

@engcom-Delta engcom-Delta self-assigned this Feb 17, 2020
@engcom-Delta
Copy link
Contributor

✔️ QA passed

@engcom-Echo engcom-Echo self-assigned this Feb 17, 2020
@magento-engcom-team magento-engcom-team mentioned this pull request Feb 21, 2020
Closed
magento-engcom-team pushed a commit that referenced this pull request Feb 21, 2020
@VladimirZaets
ENGCOM-6901: [#25963] Fixed grids export: option labels are taken fro…
88dedff 
…m grid filters and columns now. #26523
@magento-engcom-team magento-engcom-team merged commit dd89574 into magento:2.4-develop Feb 21, 2020
@m2-assistant
Copy link

m2-assistant bot commented Feb 21, 2020

Hi @novikor, 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.

@novikor novikor deleted the 25963-grid-export-labels branch February 24, 2020 10:29
@sdzhepa sdzhepa mentioned this pull request May 9, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nuzil nuzil nuzil approved these changes

@melnikovi melnikovi Awaiting requested review from melnikovi

Assignees

@nuzil nuzil

@engcom-Delta engcom-Delta

@engcom-Echo engcom-Echo

Labels
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Component: Sales Component: Ui Progress: accept Release Line: 2.4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@novikor @magento-engcom-team @engcom-Delta @nuzil @engcom-Echo