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

Chris Burge created an issue. See original summary.

geek-merlin’s picture

Quick thought: We might unset all NULLed keys on save.

chris burge’s picture

I forgot the first part of how SectionComponent::get() and SectionComponent::set() work. They check for class properties first, before interacting with $additional. So we can use them to unset a provider's $additional data.

Assuming $component is a SectionComponent object and we're wanting to remove data from provider that is keyed 'component_attributes':

$additional = $component->get('additional');
unset($additional['component_attributes']);
$component->set('additional', $additional);

I think this might best be handled by documenting in the change record when $additional is deprecated.

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?

geek-merlin’s picture

> 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.

chris burge’s picture

Status: Active » Closed (works as designed)

Agreed