Skip to content

ValidationHooks: Remove a FIXME note

Urbanecm requested to merge remove-fixme into master

Why: The FIXME note was asking to rewrite the hook implementation in a way that doesn't involve constructing a provider.

I think such a rewrite would be difficult to do without keeping the code reasonably generic. In addition to that, I don't really see much added benefit in doing so. Constructing a provider happens at most once in each request. Not constructing it in the hook could help, but only if there was nothing else using community configuration in that request.

I find that unlikely, considering one of the first CC2.0 users, Growth team itself, needs most of CC2.0-provided data on each (logged in) pageview, which is considerably more frequent than JsonValidateSave runs.

If constructing the providers within JsonValidateSave is a problem, then it is likely also a problem with constructing it on every pageview. In that case, we really need to fix whatever is causing that problem, instead of figuring a way of not constructing it in the hook.

I consider the instanceof-based approach a nicer one than directly relying on a config value, since it keeps the configuration providers the only place where CommunityConfigurationProviders needs to be parsed.

What: Remove the FIXME note from ValidationHooks, as it does not need to be done.

Merge request reports