Fix removeAdUnit

[benjaminclot]

Type of change

• Bugfix

Description of change

When there are multiple adUnits with the same code, only one of them gets removed. This fixes that.

[benjaminclot]

All tests pass but CircleCI keeps returning timeouts :-/

[idettman]

I've reviewed and the code looks ok, but I'm unable to get the circleci tests to pass.

Can you try updating your branch with the latest from Prebid.js master, it may fix to the circleci failure.

[benjaminclot]

@idettman The branch has been updated with master and there's still a timeout error.

[idettman]

LGTM

[snapwich]

This is a bug. The original code should work except they're iterating over the object they're mutating. Your solution probably works but changes functionality a little bit (creates a new array reference). I'd rather the bug was fixed without changing the exposed API (even if slight, it has potential to break things).

In this case, the fix would be to iterate over the adUnit array backwards.

[idettman]

Nice catch @snapwich, so would the fix be something like this?

  adUnitCodes.forEach((adUnitCode) => {
    for (let i = $$PREBID_GLOBAL$$.adUnits.length - 1; i >= 0; i--) {
      if ($$PREBID_GLOBAL$$.adUnits[i].code === adUnitCode) {
        $$PREBID_GLOBAL$$.adUnits.splice(i, 1);
      }
    }
  })

[snapwich]

@idettman should be i >= 0, but other than that I think it's good.

[benjaminclot]

@snapwich @idettman requested change has been committed

[mkendall07]

LGTM

[snapwich]

LGTM