Problem
Some constraint validator tests make use of a bad variable name, which is mixing up typed data objects (usually named $typed_data) and typed data manager (wrongly named $typed_data). This was previously mentioned in reviews, so this issue was created.
Steps to reproduce
-
Proposed resolution
Name the variable holding the typed data manager $typedDataManager (camel case since class property)
Remaining tasks
-
User interface changes
-
Introduced terminology
-
API changes
-
Data model changes
-
Release notes snippet
- (not need to be mentioned separately)
Original report:
Let's fix this for all of the tests of https://drupal.org/node/2012776 that have been already committed, once the follow-up for #2142989: Test for BundleConstraintValidator got committed.
Issue fork drupal-2169467
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 2169467-improve-typeddata-variable
changes, plain diff MR !12953
Comments
Comment #1
fagoComment #2
mgiffordComment #3
mgiffordComment #17
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #18
smustgrave commentedOne more bump
Comment #19
fagothis is about this comment: https://www.drupal.org/project/drupal/issues/2142991#comment-8309595
The local variable $this->typedData is named very confusingly and should be $this->typedDataManager. Those couple of tests have this issue, no biggie at all, but makes sense to fix.
Comment #21
fagoI created a quick fix for that. I fixed it for all the tests where I found the issue (checking various constraint validator tests).
Disclosure: I used AI to help me with this.
Comment #22
smustgrave commentedThanks for picking up! And announcing using AI to help, that's very appreciated.
Can we clean the issue summary update to use the standard template please.
Comment #23
fagoSure! Updated it.
Comment #24
fagoComment #25
fagoComment #26
smustgrave commentedThanks! Changes look good.
Comment #27
quietone commentedComment #28
quietone commentedActually it is for the manager.
Comment #30
quietone commentedI applied the diff and searched for more instances with
$ git grep -B5 'typedData;'. There were 6 instances, none of which were for the typed data manager. None of the tests as base classes and I search any way for usages in contrib and didn't find anything.Committed 56ee638 and pushed to 11.x. Thanks!