I believe commitqueue.js is doing the right thing for label change events. I *think* there is a bug in PG's gr-change-view.js that only updates the change when approvals are removed but not when recommendations are removed.

For posterity, started in https://gerrit-review.googlesource.com/c/103930/

I don't think that CL is the right fix for this. The CQ plugin already listens for the "labelchange" events. The handler is passed a change object which should include the new label values.

I see three possibilities:
1) The handler is never passed the new label values, and we only think the cq plugin works because it has its own poller built in.
2) The handler is always passed the new label values, and something else is going wrong.
3) The handler is only passed the new label values when approvals change, but not when recommendations change. This makes little sense to me. It's not clear how changing the body of _handleLabelRemoved would fix this case (especially since we also want to update the button if someone *else* starts a dry run, so we want to make sure we're noticing when the CQ+1 label is added, too).

"I don't think that CL is the right fix for this. The CQ plugin already listens for the "labelchange" events. The handler is passed a change object which should include the new label values."

The CQ plugin does listen for the labelchange event. But the change object does not include updated label values when a recommended value is removed, which is what that CL is trying to fix.

I just witnessed the exact opposite thing happen -- the +1 label stays, but the cq text changes.

The reason for this goes a bit deeper than just the fix in that CL, and that fix won't completely address the issue.

The thing doing the polling for change updates is the CQ plugin, not PolyGerrit itself. As far as I can tell, there's no way exposed right now for plugins to modify the data PolyGerrit uses to render things like labels, so this will be out of sync until we implement that into PG.

On the (maybe) bright side, I haven't been able to replicate the behavior in the OP, and it seems like when a user manually removes the CQ vote the button changes too.

"The thing doing the polling for change updates is the CQ plugin, not PolyGerrit itself. As far as I can tell, there's no way exposed right now for plugins to modify the data PolyGerrit uses to render things like labels, so this will be out of sync until we implement that into PG."

I added the polling in https://bugs.chromium.org/p/chromium/issues/detail?id=661312
The polling should be orthogonal to the issue here. I do not see the connection (I might be missing something) ?.
Once we remove the CQ+1 vote, the plugin which listens for label changes (separate from the poller) expects to get the right state when it redraws the buttons. It does not get the right state for recommended values (CQ+1), but it does for the approval values (CQ+2).

"On the (maybe) bright side, I haven't been able to replicate the behavior in the OP, and it seems like when a user manually removes the CQ vote the button changes too."
I can reliably reproduce this behavior 100% of the time on both chromium-review and skia-review. I guess you have to have trybot access to be able to reproduce it?

Hmm, sorry, I'm not getting notification emails from this thread.

As far as I can tell, you aren't actually removing the vote from the _change object within gr-change-view. You are fetching the change object from the backend and performing actions based on that. There's nowhere in the commit queue plugin where you're modifying the actual state of the labels in the change-metadata, correct?

When you say 'Once we remove the CQ+1 vote', what specifically do you mean? Are you referring to when a user manually removes the CQ+1 vote? 

"Hmm, sorry, I'm not getting notification emails from this thread."
Added you to CC list.

"When you say 'Once we remove the CQ+1 vote', what specifically do you mean? Are you referring to when a user manually removes the CQ+1 vote? "

Right, when a user removes the CQ+1 vote the button label does not change. Sorry if I was not clear about that. The plugin just listens for the label change event and redraws buttons after that according to the information in the change object. For CQ+1 the change object does not contain the information the plugin expects because it was not refreshed when recommended values are removed.

Thanks!

Ah, okay, I think I understand the issue now. 

When we delete votes in PolyGerrit (via the (x) on the vote in the main change screen), we only remove the vote from the 'all' category. In the CQ plugin, you look at specific categories.

This is a quick bugfix, I'll make the change and add you as a reviewer.

Awesome, thanks!

Kasper's change is out for review: https://gerrit-review.googlesource.com/c/104034/

Kasper's change has landed and been deployed. Ravi, can you verify that this is fixed?

I still see the issue (atleast on skia-review). Will have to investigate more..

I do not think the change has been deployed yet.

I can confirm what Ravi is saying, we're currently blocked on a somewhat problematic backend deployment.

The fix is now live. Tested it with https://skia-review.googlesource.com/c/10298/

Kasper fixed it but I cannot assign him as the owner because this is /p/chromium (see https://bugs.chromium.org/p/chromium/issues/detail?id=719927#c18 )

Marking as fixed.