Core 51328 - fix flickering issue

[ninaceban]

Description

Personalization flickering issue when having multiple sequencing sendEvent calls:
If we have 2 sendEvent calls:

1. sendEvent call with renderDecisions: true
2. sendEvent call with renderDecisions: false
   Second call response returns first and unhides the body.

The proposal is to unhide the body only when receiving response from a call with renderDecisions:true.

Related Issue

Motivation and Context

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Improvement (non-breaking change which does not add functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• I have signed the Adobe Open Source CLA or I'm an Adobe employee.
• I have made any necessary test changes and all tests pass.
• I have run the Sandbox successfully.

[Aaronius]

This isn't a big deal, but consider switching out !isNonEmptyArray(unprocessedDecisions) for unprocessedDecisions.length. We know unprocessedDecisions will always be an array and !isNonEmptyArray involves more mental load than necessary (it's a double negative and can also lead the reader to wonder whether we're checking if unprocessedDecisions has items or whether unprocessedDecisions is not an array or both).

[Aaronius]

I noticed you have another condition with renderDecisions down below. We may want to consolidate all this. If I understand the logic correctly, maybe something like this?

let unprocessedDecisions = response.getPayloadsByType(DECISIONS_HANDLE);

if (renderDecisions) {
  let renderableDecisions;
  ([renderableDecisions, unprocessedDecisions] = extractDecisions(unprocessedDecisions));
  if (renderableDecisions.length) { // Not sure if you need this if statement. If you need it all, it would probably be best to move it to inside executeDecisions
    executeDecisions(renderableDecisions);
  }
  showContainers();
}

return { decisions: unprocessedDecisions };

[Aaronius]

Have these been thought through:

Do we always want to add decisions to the object that customers receive in response to executing a sendEvent command? Are we doing so currently? Is the value always an array? Do we have tests that ensure that this is the case?

[ninaceban]

we return in the sendEvent promise the decisions array that contains decisions that can't be rendered by us. decisions is always an array.

[Aaronius]

It might be good to have a functional test for this. Specifically, if no decisions are returned that can't be auto-rendered, ensure that the result object that comes through the promise still has a decisions array. It would also be helpful to update the personalization documentation to show that decisions will always be provided and will always be an array. Our documentation currently suggests that result.decisions is only sometimes provided, because we have this example that checks if result.decisions exists:

alloy("sendEvent",{
    xdm:{...},
    decisionScopes:['demo-1', 'demo-2']
  }).then(function(result){
    if (result.decisions){
      // Do something with the decisions.
    }
  })

[ninaceban]

decisions will always be provided if personalization on the call.

[Aaronius]

Help me understand this: If a customer only executes one sendEvent command and renderDecisions is set to false, then the containers won't be shown until the timeout is hit? Is that what we want? Do we have tests that cover this? How do customers show the containers if they're "manually" rendering decision content?

[ninaceban]

We need the prehiding snippet to be added only if bundle is loaded async, they care about flickering, and they have sendEvent with renderDecisions: true.
If they don't have a sendEvent command with renderDecisions: true on a page, they don't need this prehiding snippet.
If they load the bundle sync they also don't need the prehiding snippet, because all the prehiding/unhiding manipulations are done by us and they don't have to worry about it.

[Aaronius]

Got it. Thanks.

[Aaronius]

Can we make this clearer in the documentation? Currently, it states:

When the SDK is loaded asynchronously, it is required to use the prehiding snippet.

[ninaceban]

yes I will, I have a special ticket for that.

[jonsnyder]

unprocessedDecisions will always be an array, so you can just do

if (unprocessedDecisions.length === 0) {

[ninaceban]

thank you

[jonsnyder]

No need for this util because unprocessedDecisions will always be an array.

[ninaceban]

thank you, make sense

[jonsnyder]

can you add this line back in, but change it to "not.toHaveBeenCalled()". Following Test Driven Design, you want the test to fail before making the change.

[ninaceban]

ok, I have added it