Handle multi-value fields correctly.

I've run across a concrete case of this issue recently, so I'll see if I can make this improvement.

The case is where the source field is configured to allow maximum 2 values (user references), and the computed token field is populated incorrectly. It seems to *always* put 2 values in (even if the source field only has 1), and always the *first* user referenced in the source field. This seems to point the way to handling this better, as it seems like there's some "awareness" of the multiple values, but we're not iterating over the source values or something.

Update: I spent awhile pairing on this today with @star-szr, who helpfully volunteered to review together. Unfortunately we didn't make much headway, but I wanted to capture some of the work we did and possible leads to pursue.

First of all, we started from a WIP patch I'd rolled which looks up the cardinality of the computed token field, and then attempts to render the token $cardinality times, rendering the token repeatedly but switching out the $delta value. I'll try to distill that patch to its key points and push on an MR shortly.

This general approach seems like it ought to be viable, although we noted a tricky point from a usability standpoint: this assumes that the token will accept a delta (ie. points to a field that has multiple values), and silently fails if that's not the case. We couldn't see a way to introspect or otherwise infer whether a given token "accepts deltas" or even identify that a token refers to a field (so that we might look up its cardinality ourselves), which could allow a mechanism to throw an error. We alternately considered attempting to render the token without *clearing* the string, and then check if the rendered value matches the original, and treat *that* as the error condition. Another possibility might be to just provide a "debug mode" to the module such that a site-builder can troubleshoot their field configs by being able to see what Computed Token Field is doing (ie. show a message when computing a field, show its name, token and rendered value). At any rate, we set this aside for the time being, and honestly it may suffice to just have some good documentation about cardinality and multiple values.

In terms of the bug itself, what we're seeing with the patch attempting to handle multiple values is that the computed value we return seems to be empty (the tokens aren't rendering anything), and that empty value is not being handled well, resulting in a 500 error, where the stack trace indicates a failure in validating the form (in the case of an entityreference computed token field), or saving the field value (in the case of a simpler plain text field).

Tracing this in a debugger, we were able to see the token rendering empty values, and for awhile it seemed that it may be related to the fact that the content entity is new, and so the computed token referred to a value not in the database yet. However, reverting back to the original code it appears that the computed value renders correctly even on an initial save (we should doublecheck this more carefully).

To the extent that the module should handle empty values correctly, this is somewhat beside the point, but looking at the code flow in this context helped to illuminate the problem somewhat. I suspect the problem lies with the "shape" of the values returned changing with my patch applied. In the original code we return a string for a Text field, whereas now we return an array. This is probably a good change, but I think some of the logic further upstream doesn't account for it. We could see that we ended up in the parent ComputedFieldItemTrait's isEmpty() method, which clearly doesn't anticipate receiving an array of values. So possibly overriding that function to handle that better could help here.

On the other hand, I dug up some old links in the Computed Field module that suggest a possibly different direction. This docs page → describes how to handle multiple values in Computed Field, and indicates (for Drupal 8+) that the computed function will get called once for each $delta in its cardinality. this issue in Computed Field's queue → shows some of the reasoning and implementation for handling this in an unlimited value configuration. It's not clear to me whether we inherit this "call the computed function for each delta" behaviour from Computed Field, but if so, it might explain why the approach above isn't working, and if not, perhaps it's something to pursue. It'd certainly help to have the delta passed to our compute function, and just include that in the token to render.

Final note: just at the tail end of our debugging session, we noticed that there are 2 variations of token that includes the delta one is like [node:field_example:0], and the other is [node:field_example:0:value]. I'm not certain what the difference is, though my hunch is that in most cases we want the value variation. Although possibly this varies by field type? Another piece to consider.

That's all for now: I plan to roll my patch into an MR to share it here, and then set this aside for the moment. I intend to work on 📌 Implement localdev setup and DrupalCI test harness Active next, to make it easier to work on bugs like this.