icingaweb2 icon indicating copy to clipboard operation
icingaweb2 copied to clipboard

add canceled downtimes to the history, if they were started

Open Robnarok opened this issue 1 year ago • 9 comments

fixes https://github.com/Icinga/icingaweb2/issues/5176

Robnarok avatar Feb 09 '24 10:02 Robnarok

Thank you for your pull request. Before we can look at it, you'll need to sign a Contributor License Agreement (CLA).

Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA.

After that, please reply here with a comment and we'll verify.

Contributors that have not signed yet: @Robnarok

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Please contact us if you think this is the case.

  • If you signed the CLA as a corporation, your GitHub username may not have been submitted to us. Please reach out to the responsible person in your organization.

cla-bot[bot] avatar Feb 09 '24 10:02 cla-bot[bot]

i have signed the CLA

Robnarok avatar Feb 09 '24 10:02 Robnarok

@cla-bot check

bobapple avatar Feb 09 '24 10:02 bobapple

@gianlucapiccolo @mcodato This is the exact opposite of https://github.com/Icinga/icingaweb2/commit/db9888b1f1380355c68d759b59844e17b0241043. Please have a look here and add your thoughts.

nilmerg avatar Feb 09 '24 10:02 nilmerg

i took another look at the implementation, and if i understand it correctly, the current behavior is:

if a downtime has started and is not cancelled it is displayed
if a downtime has started and is cancelled it is NOT displayed
if a downtime has NOT started and is cancelled it is NOT displayed
if a downtime has NOT started and is NOT cancelled it is NOT displayed

removing the AND part like i did in the current PR, it would lead to

if a downtime has started and is NOT cancelled it is displayed
if a downtime has started and is cancelled it is displayed
if a downtime has NOT started and is cancelled it is NOT displayed
if a downtime has NOT started and is NOT cancelled it is NOT displayed

or a third solution could be if we would replace the AND with an OR (was_started=1 OR was_cancelled=0) we would get something like

if a downtime has started and is NOT cancelled it is displayed
if a downtime has started and is cancelled it is displayed
if a downtime has NOT started and is cancelled it is NOT displayed
if a downtime has NOT started and is NOT cancelled it is displayed

(Edit: And prior to https://github.com/Icinga/icingaweb2/commit/db9888b1f1380355c68d759b59844e17b0241043 all of the 4 cases were displayed in the history)

Robnarok avatar Feb 09 '24 14:02 Robnarok

Hi @Robnarok, I see your point, the change we made was for another very annoying bug, let me do some internal tests and talk about it with my colleagues and I will let you know as soon as possible, thank you

mcodato avatar Feb 09 '24 15:02 mcodato

@Robnarok Thank you for your patience, the tests showed that your PR is fine for us, as it keeps the was_started = 1 :)

mcodato avatar Feb 12 '24 15:02 mcodato

@Robnarok Please rebase with main, this should fix the actions

nilmerg avatar Feb 12 '24 15:02 nilmerg

@mcodato Thank you for testing :)

@nilmerg i did the rebase and the actions passed :partying_face:

Robnarok avatar Feb 12 '24 16:02 Robnarok