ConfigurableLanguage class variables should not be accessed directly. Functions should be use to access the variable. For instance use getDescription() and setDescription($description) for the protected class variable description. For a boolean variable the getter function becomes isVariableName(). In object-oriented programming this is called encapsulation.
Remaining tasks
- Update the class variables and make them protected.
- Create getters and setters for frequently used get and set functionality.
- Update drupal to use the getters and setters instead of accessing variables directly.
- There are no tests required because the added functions are only getters and setters.
For more info over what should be done see the issue summary of #2016679: Expand Entity Type interfaces to provide methods, protect the properties.
Beta phase evaluation
Issue category | Bug because properties should not be public, API methods should not be allowed to be sidestepped. |
---|---|
Issue priority | Major because this meta goes across the entire system. But each child will be a normal bug. |
Prioritized changes | Prioritized since it is a bug and it reduces fragility. |
Disruption | Somewhat disruptive for core as well as contributed and custom modules:
|
But impact will be greater than the disruption, so it is allowed in the beta.
Comment | File | Size | Author |
---|---|---|---|
#5 | 2384541-5.patch | 539 bytes | rpayanm |
Comments
Comment #1
daffie CreditAttribution: daffie commentedI would like to get this fixed. So I will do a good review for posted patches.
Comment #2
fernando_calsa CreditAttribution: fernando_calsa commentedI've seen the file core/modules/language/Entity/ConfigurableLanguage.php and the only property what is public is $label what is the human-readable lable for the language, but the rest of the properties are protected and they have getters and setters, perhaps I am confused
Comment #3
daffie CreditAttribution: daffie commentedIf only the $label variable is public, then that is the only one that needs to change. There is already a build in function label(). The only problem that I can see is that somewhere a ConfigurableLanguage class instance accesses the label variable directly and that will result in errors. Those need to be fixed. And that is all there is to do for this issue. You can do patch with the only change being that the label variable is set to protected and run the tests and see what goes wrong.
Comment #4
daffie CreditAttribution: daffie commentedUpdated the issue summary with link to parent issue for more info.
Comment #5
rpayanmLet me see...
Comment #6
daffie CreditAttribution: daffie commentedAll the class variable label is protected.
The getter function is already available, so no need for a new one.
The test-server give it green.
It all looks good to me, so for me it is RTBC.
Comment #7
rpayanm@daffie would be good adding getLabel() and setLabel() to this class ?
Comment #8
daffie CreditAttribution: daffie commented@rpayanm: The function getLabel() is not used. We have a build in function called label() that does the same. It is inherited from the Entity class. The function setLabel() is not added because it will most probably almost never used.
Comment #9
rpayanm@daffie ok, thank you!
Comment #10
alexpottCommitted 7f902cb and pushed to 8.0.x. Thanks!
Thanks for adding the beta evaluation for to the issue summary.