@AndyRussG you fixed this with https://gerrit.wikimedia.org/r/523095 , right?

In T229365#5422587, @Ejegg wrote:

@AndyRussG you fixed this with https://gerrit.wikimedia.org/r/523095 , right?

No, I'm afraid not. All that did was move the existing logic of the very fallible check elsewhere, and add a status for when it fails. However, no calls to recordImpression() are made in this case (nor any other case when no campaign is chosen).

So, the code is clearer and easier to debug if a user can check the JS console, but no data is sent back, and the check is still very flimsy.

Regarding the flimsiness of the check, it's very possible that clients could get stale choiceData, so incorrect settings for campaigns, but without that being detected by the current code, since to detect the problem, a campaign in the choiceData has to have already expired.

It just seems like it would be super simple to include, say, a timestamp with the all the choiceData and check that instead. It'd be an easy win, and would help ensure that if a banner has to be taken down in an emergency, clients won't keep seeing it.

Adding calls to recordImpression() in that case might require a bit of planning, but probably very little additional code.

Thoughts?

Thanks much!!