Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Currently, when the key add/edit form is submitted, if there's a key value to be set, it's done as part of the form submission handler. This complicates setting key values in cases where a key is being created programmatically and not through the admin UI.
Proposed resolution
The functionality to set the key value should be encapsulated in the Key entity. If setting the key value fails, an exception should be thrown.
Remaining tasks
- Add functionality to set a key value to the preSave method of the Key entity
- Define a custom exception for when the key value is not set successfully
- Throw the exception when the key value is not set successfully
- Catch the exception in the Key entity and display an error
- Remove the functionality to set a key value from the add/edit form submit handler
User interface changes
There are no changes that would be visible to the user.
API changes
The Key entity would provide support for set key values, but it should not affect any currently implemented methods for setting the key value directly through the key provider.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | interdiff.txt | 598 bytes | rlhawk |
| #10 | 2693145-10.patch | 9.96 KB | rlhawk |
| |||
| #8 | interdiff.txt | 1.62 KB | rlhawk |
| #8 | 2693145-8.patch | 9.74 KB | rlhawk |
| |||
| #7 | 2693145-7.patch | 9.72 KB | rlhawk |
| |||
Comments
Comment #2
rlhawkComment #3
discipolo CreditAttribution: discipolo commentedi may be totally off track here but would it make sense to add and add() method to the repository?
since we don't only need to set the value from somewhere else but also create new key entities
see suggestion at the bottom of http://php-and-symfony.matthiasnoback.nl/2014/05/inject-a-repository-ins...
Comment #4
rlhawk@discipolo - That sounds intriguing. Do you know of any Drupal core modules have that functionality, or are going to be moving in that direction, in order to match the Symfony pattern?
Comment #5
rlhawkHere's a patch that adds saving a key value to the Key entity. The configuration key provider is updated to throw an exception if setting the key value fails.
Comment #6
rlhawkI have some more work to do on this, so marking postponed.
Comment #7
rlhawkHere's a new patch, along with an interdiff.
The only difference between this and the previous patch is more checking for a key value when the entity is saved. If the provider supports setting a value and requires one, but no value is defined, an exception is thrown. If the provider does not support setting a value, but one is defined, an exception is thrown.
Comment #8
rlhawkI realized my logic was a bit faulty when determining if a key value was empty, so I've updated it. Here's a new patch, along with a new interdiff (from the original patch).
Comment #9
tynor CreditAttribution: tynor commentedTook a look at the patch and I noticed a bug in the validation logic.
Steps to reproduce:
Expected result:
Key is saved with a blank key value (key value for Configuration is not a required field).
Actual result:
Error is reported for not being able to save a key value.
I'm not 100% positive whether this is the desired behavior, but it is not what I expected because of the lack of requirement on the key value field.
Comment #10
rlhawkThanks, Tynor. Here's a new patch and an interdiff from the previous patch.
Comment #11
tynor CreditAttribution: tynor commentedGreat, looks good to me.
Comment #13
rlhawkThanks!