Problem/Motivation
With JSON:API allowing to disable fields through an event subscribe an application could have a (contrib) module that disables a field in a resource. If you want to enable that field again in your resource responses this currently isn't possible.
Steps to reproduce
Add an event subscriber that disabled a field. There is now no way to re-enable that field without any weird magic.
namespace Drupal\jsonapi_test_resource_type_building\EventSubscriber;
use Drupal\jsonapi\ResourceType\ResourceTypeBuildEvents;
use Drupal\jsonapi\ResourceType\ResourceTypeBuildEvent;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
/**
* Event subscriber which tests disabling resource types.
*
* @internal
*/
class ResourceTypeBuildEventSubscriber implements EventSubscriberInterface {
/**
* {@inheritdoc}
*/
public static function getSubscribedEvents(): array {
return [
ResourceTypeBuildEvents::BUILD => [
['disableResourceTypeFields'],
],
];
}
/**
* Disables any resource type fields that have been aliased by a test.
*
* @param \Drupal\jsonapi\ResourceType\ResourceTypeBuildEvent $event
* The build event.
*/
public function disableResourceTypeFields(ResourceTypeBuildEvent $event) {
foreach ($event->getFields() as $field) {
// Disable the internal Drupal identifiers.
if (str_starts_with($field->getPublicName(), 'drupal_internal__')) {
$event->disableField($field);
}
}
}
}
Proposed resolution
Propose we add the possibility to enable fields again in the ResourceTypeBuildEvents.
Remaining tasks
Implement the proposed resolution by addingenableFieldto the EventSubscriber andenabledto ResourceTypes.Write testsAdd change record- Follow up to update documentation
User interface changes
n.a.
API changes
New method enableField on ResourceTypeBuildEvents and new method enabled on ResourceType to modify the resource type.
Data model changes
n.a.
Release notes snippet
n.a.
Issue fork drupal-3110831
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:
Comments
Comment #2
xjmComment #5
bbralaThis is a valid point. Adding to #3032787: [META] Start creating the public PHP API of the JSON:API module.
Comment #8
bbralaThis annoyed me so spent a little time implementing. Implementation was ealy enough.
Comment #9
bbralaComment #10
bbralaComment #13
wim leersOnce there's a change record, this is insta-RTBC IMHO?
Comment #14
bbralaI would say, if you have looked at the code, I can easily write a CR tomorrow.
Comment #15
wim leersCan you write the CR and address the nits? Then this is RTBC 🤓
Comment #16
z3cka commentedI took a stab at writing a change record for this: https://www.drupal.org/node/3319533 Please, any feedback welcome.
I'm going to go ahead and test the MR in my current project, as this is just what I need right now. Cheers!
Comment #17
z3cka commentedComment #18
bbralaThanks for CR and the testing. I've tweaked the CR a little so its a bit more expressive and has your example. I'll now work on the few nits that @Wim Leers has mentioned.
Comment #19
bbralaI've addressed all the little nits. Also retargeted to 10.1.x and rebased. Thanks everyone. Setting back to NR :)
Comment #20
bbralaComment #21
smustgrave commentedThis issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.
Verified the test coverage running locally without the fix. I got
Error : Call to undefined method Drupal\jsonapi\ResourceType\ResourceTypeBuildEvent::enableField()
Which is good!
All threads are resolved on the MR
All tests are green
Change record is added and make sense (to me at least)
New functions are typehinted
This looks good to me.
Comment #22
alexpottThere are several other options here. You could alter the service that subscribes the event or you could implement an event subscriber with a higher priority and stop propagation. I'm not sure that adding
enableField()to fight it out withdisableField()is going to improve the situation. I can imagine people adding an event to disable a field, that was disabled and then enabled. Feels odd.Comment #23
smustgrave commentedAlright issue should be reworked based on #22 or explained why this is the better approach.
Decision should be documented in issue summary please
Thanks.
Comment #25
matthandIs there a way to re-enable the UUID field that is disabled on node resources by JSON:API without this new enableField method? I don't think so. Real world scenario of trying to match a legacy Drupal 8 JSON:API endpoint to prevent consumer apps requiring a rebuild.
Comment #27
z3cka commentedI agree that this can probably be handled better, but until a better solution is provided, I went ahead and rebased the temp fix on to the Drupal
10.3.xbranch for those that need a compatible patch.Comment #28
bbralaI really think this is the way, it might need some priority juggling to make work in your application but that should be fine. I'll update the MR to 11.x and make sure codestyle and everything is up to date.
Comment #29
bbralaMoved all changed to 11.x, also updates a return type that didnt make sense. I think this is good like this. I've updated the issue summary also.
I still think this is the best option for adjusting, even though this could mean you enable and disable multiple times. But i don't think this will be an actually issue in implementations.
Comment #30
smustgrave commentedReading the CR and it appears to contain good detail with a nice example
Test-only feature has already been ran
So coverage is there.
Added some super nitpicky :void returns directly.
Believe all feedback has been addressed.
Comment #33
longwaveCommitted to 11.x for 11.1.0 and backported to 10.4.x as a minor API addition to keep things in sync. Also published the change record.
Committed and pushed a1c6ae78c3 to 11.x and 257fb3b34b to 10.4.x. Thanks!
Comment #34
wim leersNice, one more thing in #3032787: [META] Start creating the public PHP API of the JSON:API module that ships! 🥳
Comment #35
bbralaWe'll get there, step to step.