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

\Magento\Ui\Component\Listing\Columns\Column has access to all grid columns #6525

Closed
Dublerq opened this issue Sep 7, 2016 · 4 comments
Closed
Labels
bug report Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release triage wanted

Comments

@Dublerq
Copy link

Dublerq commented Sep 7, 2016

You can use an extended column class instead of the default \Magento\Ui\Component\Listing\Columns\Column in a grid ui component which is great. Less great thing is that when you set a custom class for one column like:

<column name="required" class="\Vendor\ModuleName\Ui\Component\Listing\Column\Yesno">

In public function prepareDataSource(array $dataSource) you get unlimited access to data of all columns.

Preconditions

  1. Tested on Magento 2.1.1

Steps to reproduce

  1. Create an extended column class
  2. Dump content of $dataSource in method public function prepareDataSource(array $dataSource)

Expected result

  1. An array with element related only to this certain column

Actual result

  1. An array with full access to all data columns.

I just hope that programmers will not overuse this access and keep their code clean.

Note: changing this behavior may brake many already existing scripts

@heldchen
Copy link
Contributor

heldchen commented Sep 8, 2016

how would one specify which data elements are needed for this column? and in this case, what is keeping someone from requesting all columns? in my case, we have some extra columns that depend on other data than just the field that they are representing...

@Dublerq
Copy link
Author

Dublerq commented Sep 8, 2016

how would one specify which data elements are needed for this column?

It should pass only the data of that one column, or give read only access to all columns and r/w to data from specified one.

and in this case, what is keeping someone from requesting all columns? in my case, we have some extra columns that depend on other data than just the field that they are representing...

It's ok to have read access to other columns, but no the write one. Editing other columns data in class of that specified, one column is a terrible feature leading to badly written code and total mess when overused.

@magento-engcom-team magento-engcom-team added 2.1.x bug report Area: Frontend Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed labels Sep 11, 2017
@magento-engcom-team magento-engcom-team added Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed 2.2.x and removed Area: Frontend labels Nov 27, 2017
@magento-engcom-team
Copy link
Contributor

@Dublerq, thank you for your report.
We've created internal ticket(s) MAGETWO-84572 to track progress on the issue.

@magento-engcom-team magento-engcom-team added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Nov 27, 2017
@magento-engcom-team
Copy link
Contributor

@Dublerq, thank you for your report.

Unfortunately, we are archiving this ticket now as it did not get much attention from both Magento Community and Core developers for an extended period. This is done in an effort to create a quality, community-driven backlog which will allow us to allocate the required attention more easily.

You may learn more about this initiative following this link.

Please feel free to comment or reopen the ticket if you think it should be reviewed once more. Thank you for collaboration.

magento-engcom-team pushed a commit that referenced this issue Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release triage wanted
Projects
None yet
Development

No branches or pull requests

5 participants