UI shows scheduled deletions for unrelated channels (SOLVED)

[AMUZY]

Hi @gabrielBusta , this PR solves the issue about scheduled deletions showing for unrelated channels

• Before fix :

• After fix

[AMUZY]

I also want to ask, is it done on purpose that if we filter the Firefox product by a channel release-cdntest, we still see all the release and release* channels showing up ?

[bhearsum]

I also want to ask, is it done on purpose that if we filter the Firefox product by a channel release-cdntest, we still see all the release and release* channels showing up ?

You should not be seeing release rules on release-cdntest. You should be seeing release* - which is a glob that matches anything starting with release. (Note that if a Rule has a scheduled change pending, it will show up if either the current version of the Rule or the scheduled change matches the channel filter, eg: if either contains release* it will show up for all channels that start with release.)

[AMUZY]

Yeah you're right. It's just the release* channel that shows up. But please have you tested it out yet? @gabrielBusta

[AMUZY]

Hi @bhearsum , I have refactored the conditions to better match what is required of the filter function and it works. Please take a look thanks.

[AMUZY]

Apologies that I haven't been able to give this a full review yet - I will try to get to it tomorrow. One quick comment: you should use

https://github.com/mozilla-releng/balrog/blob/a4536abd23c8e9d3d103f6fae1c10ef593fe58db/ui/src/utils/rules.js#L2

to compare the rule to a channel rather than doing it by hand here. (As you can see, that function covers even more cases as well.)

I have tried this function on two occasions here and it didn't work perfectly in this case. It doesn't consider the product, rather checks only with the channel. It's suitable for other cases just not here. I'd try it again and give you a feedback later.

[bhearsum]

Apologies that I haven't been able to give this a full review yet - I will try to get to it tomorrow. One quick comment: you should use https://github.com/mozilla-releng/balrog/blob/a4536abd23c8e9d3d103f6fae1c10ef593fe58db/ui/src/utils/rules.js#L2

to compare the rule to a channel rather than doing it by hand here. (As you can see, that function covers even more cases as well.)

I have tried this function on two occasions here and it didn't work perfectly in this case. It doesn't consider the product, rather checks only with the channel. It's suitable for other cases just not here. I'd try it again and give you a feedback later.

Thanks for calling this out -- I think you're right, as this considers a scheduled change with a null channel to be a match. With that being the case, we should refactor it a bit to at least be able to make use of the matchesGlob function currently inside of it. (That's the main thing I don't want to see re-implemented elsewhere.)

[AMUZY]

Apologies that I haven't been able to give this a full review yet - I will try to get to it tomorrow. One quick comment: you should use https://github.com/mozilla-releng/balrog/blob/a4536abd23c8e9d3d103f6fae1c10ef593fe58db/ui/src/utils/rules.js#L2

to compare the rule to a channel rather than doing it by hand here. (As you can see, that function covers even more cases as well.)

I have tried this function on two occasions here and it didn't work perfectly in this case. It doesn't consider the product, rather checks only with the channel. It's suitable for other cases just not here. I'd try it again and give you a feedback later.

Thanks for calling this out -- I think you're right, as this considers a scheduled change with a null channel to be a match. With that being the case, we should refactor it a bit to at least be able to make use of the matchesGlob function currently inside of it. (That's the main thing I don't want to see re-implemented elsewhere.)

Hah! Told ya I was right. Don't worry, I'd refactor that function to accommodate the current conditions and give you feedback soon👍