For my own notes, all of this happens in state.js.

@AndyRussG Is there any documentation of what these status codes mean?

Also @AndyRussG what is the correct, expected behavior here?

@AndyRussG I tried copying and pasting that query using impyla but I'm getting:
"HiveServer2Error: Error while compiling statement: FAILED: ParseException line 4:0 missing EOF at 'SELECT' near ')'". -- This is resolved so this comment can be ignored.

In T240802#5790095, @mepps wrote:

@AndyRussG Is there any documentation of what these status codes mean?

Yep! Here is the doc. The code itself also documents this; at the beginning of state.js, see STATUSES for the overall statuses and REASONS for the reasons. The status code is just the status number, a dot, and the reason number. So, for 2.1 the overall status is banner_canceled and the reason is close, which indicates the banner was not shown because the user had closed a banner in the same category recently on a previous pageview.

what is the correct, expected behavior here?

The correct behaviour is that, with campaign fallback, the status code for a campaign fallen back from should have no impact on the status code of any subsequent campaigns that we try. The campaignStatuses property in the data we send back is an array of the campaigns chosen and their respective status codes, in the order in which they were chosen.

From the query in the task description, we can see that a non-FR campaign was correctly selected many times when the FR campaign opted not to show a banner. However, the status code from the FR campaign often carried over to the non-FR campaign. It's clear that something's wrong because the non-FR campaign sometimes got a 2.9 status code, which is banner cancelled because the user has donated. However, non-FR campaigns should never get that status code.

Hope this is useful!! Thanks for working on this!

@AndyRussG Okay, I can see what you mean. When I look at the loop what I see is that state.setCampaign is called early on and that should set the status to CAMPAIGN_CHOSEN for the campaign that gets selected. cancelBanner is called if hide.shouldHide() is true. Just recording my findings as I go.

In T240802#5797791, @mepps wrote:

@AndyRussG Okay, I can see what you mean. When I look at the loop what I see is that state.setCampaign is called early on and that should set the status to CAMPAIGN_CHOSEN for the campaign that gets selected. cancelBanner is called if hide.shouldHide() is true. Just recording my findings as I go.

Yep!! All correct :)

@AndyRussG, @Ejegg and I think we see what happened, and I've made a commit. However, I don't know how to test it. Any thoughts on that?

Change 564062 had a related patch set uploaded (by Mepps; owner: Mepps):
[mediawiki/extensions/CentralNotice@master] Reset hide.shouldHide and hide.reason when setting new category

https://gerrit.wikimedia.org/r/564062

I'm trying to test my patch locally but the banners still aren't displaying with "error: Not Found" in mw.centralNotice.data.errorMsg.

To test the fix locally:

• Set up two campaigns, one higher priority than the other.
• Include a close button in the banner for the campaign with higher priority.
• The higher priority campaign should always show, until the close button is clicked on the banner. On subsequent pageviews after the button has been clicked, the lower priority campaign should show.
• To test again, clear cookies for the local wiki.

You can also use this procedure to reproduce the bug in the current, broken version of the code: use the same setup, and click the close button in the banner of the higher-priority campaign. In the broken state, no campaigns should show on subsequent pageviews, and in campaignStatuses, both campaigns should get the banner hidden status code.

Thanks for working on this!!

Change 564062 merged by jenkins-bot:
[mediawiki/extensions/CentralNotice@master] Reset hide shouldHide,reason private variables when setting new category

https://gerrit.wikimedia.org/r/564062

Change 566563 had a related patch set uploaded (by Mepps; owner: Mepps):
[mediawiki/extensions/CentralNotice@wmf_deploy] Reset hide shouldHide,reason private variables when setting new category

https://gerrit.wikimedia.org/r/566563

Change 566563 merged by jenkins-bot:
[mediawiki/extensions/CentralNotice@wmf_deploy] Reset hide shouldHide,reason private variables when setting new category

https://gerrit.wikimedia.org/r/566563

Change 566901 had a related patch set uploaded (by Catrope; owner: Mepps):
[mediawiki/extensions/CentralNotice@wmf/1.35.0-wmf.16] Reset hide shouldHide,reason private variables when setting new category

https://gerrit.wikimedia.org/r/566901

Change 566901 merged by Catrope:
[mediawiki/extensions/CentralNotice@wmf/1.35.0-wmf.16] Reset hide shouldHide,reason private variables when setting new category

https://gerrit.wikimedia.org/r/566901

Change 566902 had a related patch set uploaded (by Catrope; owner: Mepps):
[mediawiki/extensions/CentralNotice@wmf/1.35.0-wmf.15] Reset hide shouldHide,reason private variables when setting new category

https://gerrit.wikimedia.org/r/566902

Change 566902 merged by Catrope:
[mediawiki/extensions/CentralNotice@wmf/1.35.0-wmf.15] Reset hide shouldHide,reason private variables when setting new category

https://gerrit.wikimedia.org/r/566902

Mentioned in SAL (#wikimedia-operations) [2020-01-24T00:34:48Z] <catrope@deploy1001> Synchronized php-1.35.0-wmf.15/extensions/CentralNotice/resources/ext.centralNotice.display/hide.js: T240802 (duration: 01m 07s)

Mentioned in SAL (#wikimedia-operations) [2020-01-24T00:36:02Z] <catrope@deploy1001> Synchronized php-1.35.0-wmf.16/extensions/CentralNotice/resources/ext.centralNotice.display/hide.js: T240802 (duration: 01m 05s)