Problem/Motivation
SectionComponent provides get() and set() methods for interacting with the $additional property. It does not provide an unset() method, however. This makes it impossible for a provider to remove its configuration. The provider can set its value to an empty string or NULL; however, it cannot remove its key.
Proposed resolution
Add an unset() method.
$additional doesn't have test coverage currently, and it's on track to be deprecated.. so, do we need to add test coverage?
Remaining tasks
- Submit patch
- Add tests?
User interface changes
None.
API changes
An unset() method is added to SectionComponent
Data model changes
None.
Release notes snippet
TBD
Comments
Comment #2
geek-merlinQuick thought: We might unset all NULLed keys on save.
Comment #3
chris burge commentedI forgot the first part of how
SectionComponent::get()andSectionComponent::set()work. They check for class properties first, before interacting with$additional. So we can use them to unset a provider's$additionaldata.Assuming
$componentis aSectionComponentobject and we're wanting to remove data from provider that is keyed 'component_attributes':I think this might best be handled by documenting in the change record when
$additionalis deprecated.This does raise a larger issue; however: Should
SectionComponent::get()andSectionComponent::set()continue to exist? With$additionalremoved, aren't they effectively__get()and__set()at that point?Comment #4
geek-merlin> I think this might best be handled by documenting [this code] in the change record when $additional is deprecated.
Fully agree. So this issue'd be works-as-designed.
> This does raise a larger issue; however: Should SectionComponent::get() and SectionComponent::set() continue to exist? With $additional removed, aren't they effectively __get() and __set() at that point?
I din't think we even need them any more, as all properties have getters and setters. (UUID has intentionally no setter and gy removing set() we close that hole to change UUID. :-)
So let's deprecate get() / set() in the other issue.
Comment #5
chris burge commentedAgreed