Skip to content
This repository has been archived by the owner. It is now read-only.

[dev.icinga.com #3329] committing acknowledgement cmd negates flags for send_notification and sticky_ack #1139

Closed
icinga-migration opened this issue Oct 23, 2012 · 12 comments

Comments

Projects
None yet
1 participant
@icinga-migration
Copy link
Member

commented Oct 23, 2012

This issue has been migrated from Redmine: https://dev.icinga.com/issues/3329

Created by chunghung.chen on 2012-10-23 14:55:37 +00:00

Assignee: ricardo
Status: Resolved (closed on 2012-11-10 14:41:53 +00:00)
Target Version: 1.8.2
Last Update: 2014-12-08 09:27:46 +00:00 (in Redmine)

Icinga Version: 1.8.1
OS Version: any

After upgrade to 1.8.0, when acknowledge an event in classic ui, "notification bit" is wrong.
If I checked "Send notification", in icinga.log shows that notification bit is 0.
If I un-check "Send notification", log says notification bit is 1.
Please help to confirm this problem and fix it.
Thanks and sorry for my bad English.

Changesets

2012-10-27 13:34:47 +00:00 by mfriedrich eda48c8

ugly workaround for sending (un)checked values for sticky_ack and send_notification values on acknowledgement (refs #3329)

2012-11-09 16:38:41 +00:00 by ricardo 27f47ba

Revert "ugly workaround for sending (un)checked values for sticky_ack and send_notification values on acknowledgement (refs #3329)"

This reverts commit eda48c8b21dd044ffd79f15c8c731202053abafa.

refs: #3329

found a way of not messing around with html and hoping every browser behaving the same way.

2012-11-09 17:01:29 +00:00 by ricardo ee330c1

classic-ui: fixed committing acknowledgement cmd negates flags for send_notification and sticky_ack #3329

refs: #3329

Found another way on checking if the default TRUE values for send_notification and sticky_ack are set or not.
Don't really like the idea of adding a hidden html input and hoping that every browser behaves the same.

Relations:

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 24, 2012

Updated by mfriedrich on 2012-10-24 12:38:23 +00:00

can you elaborate a bit more by example, like providing the logs themselves?

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 24, 2012

Updated by mzac on 2012-10-24 13:57:26 +00:00

I see the same thing:

With send notification checked:

[1351086922] EXTERNAL COMMAND: ACKNOWLEDGE_HOST_PROBLEM;desk-ups;1;0;0;admin;ack

With send notification unchecked:

[1351086994] EXTERNAL COMMAND: ACKNOWLEDGE_HOST_PROBLEM;desk-ups;1;1;0;admin;ack
[1351086994] HOST NOTIFICATION: icingaadmin;desk-ups;ACKNOWLEDGEMENT (DOWN);notify-host-by-email;CRITICAL - Host Unreachable (192.168.1.30);admin;ack
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 27, 2012

Updated by bear on 2012-10-27 11:50:51 +00:00

I can confirm this behavior using the latest code from Git.

Commit 8c3910c, Thu Oct 25 00:48:07 2012 +0200.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 27, 2012

Updated by mfriedrich on 2012-10-27 12:12:40 +00:00

  • Status changed from New to Assigned
  • Assigned to set to mfriedrich
  • Target Version set to 1.8.2

true with 1.8.1 as well.

with notifications checked

Oct 27 14:09:43 sol icinga: EXTERNAL COMMAND: ACKNOWLEDGE_SVC_PROBLEM;1300localhost;DISEÑOS;1;0;0;icingademo;notificationtest

unchecked

Oct 27 14:11:24 sol icinga: EXTERNAL COMMAND: ACKNOWLEDGE_SVC_PROBLEM;1300localhost;DISEÑOS;1;1;0;icingademo;notificationtest2
Oct 27 14:11:24 sol icinga: SERVICE NOTIFICATION: testconfig-admin;1300localhost;DISEÑOS;ACKNOWLEDGEMENT (CRITICAL);true;CRITICAL: foobaer;icingademo;notificationtest2
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 27, 2012

Updated by mfriedrich on 2012-10-27 12:12:59 +00:00

  • Category set to 52
  • Icinga Version changed from 1 to 1
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 27, 2012

Updated by mfriedrich on 2012-10-27 12:21:55 +00:00

from the cgi log

[1351339783] LOG ROTATION: DAILY
[1351339783] This log is highly experimental and changes may occure without notice. Use at your own risk!!
[1351339783] EXTERNAL COMMAND: icingademo;1.2.3.4;ACKNOWLEDGE_SVC_PROBLEM;1300localhost;DISEÑOS;1;0;0;icingademo;notificationtest
[1351339846] EXTERNAL COMMAND: icingademo;1.2.3.4;REMOVE_SVC_ACKNOWLEDGEMENT;1300localhost;DISEÑOS
[1351339884] EXTERNAL COMMAND: icingademo;1.2.3.4;ACKNOWLEDGE_SVC_PROBLEM;1300localhost;DISEÑOS;1;1;0;icingademo;notificationtest2
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 27, 2012

Updated by mfriedrich on 2012-10-27 12:40:46 +00:00

  • Subject changed from Classic UI send wrong command for ACKNOWLEDGEMENT to acknowledgement cmd sends negated flags for send_notification and sticky_ack

the same happens for sticky_ack as well, so this sources from #2927

my guess is, that the cmd.cgi sends the check boxes via POST, but actually the value field is omitted, leaving the default value in place.

need to test further on a possible fix.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 27, 2012

Updated by mfriedrich on 2012-10-27 13:33:58 +00:00

  • Assigned to changed from mfriedrich to ricardo

hrm. typical html problem with checkboxes - you cannot simply determine the value to send, after the client (user) changed the checkbox, if you do not want to call some javascript on submit, setting those values.

though, there's an ugly workaround for that - add a hidden html field, which pushes the value of the original name, if that one was not checked.

http://iamcam.wordpress.com/2008/01/15/unchecked-checkbox-values/

actually this works for me.

michi@sol ~/coding/icinga/icinga-core @ mfriedrich/cgis $ git diff
diff --git a/cgi/cmd.c b/cgi/cmd.c
index 1cf832b..35ba013 100644
--- a/cgi/cmd.c
+++ b/cgi/cmd.c
@@ -1010,7 +1010,9 @@ void print_form_element(int element, int cmd) {
                printf("Sticky Acknowledgement:");
                print_help_box(help_text);
                printf("");
-               printf("\n", (sticky_ack == TRUE) ? "CHECKED" : "");
+               /* http://iamcam.wordpress.com/2008/01/15/unchecked-checkbox-values/ */
+               printf("", (sticky_ack == TRUE) ? 0 : 1); /* negate value hack, if checkbox is unchecked */
+               printf("\n", (sticky_ack == TRUE) ? 1 : 0, (sticky_ack == TRUE) ? "CHECKED" : "");
                break;

        case PRINT_SEND_NOTFICATION:
@@ -1020,7 +1022,9 @@ void print_form_element(int element, int cmd) {
                printf("Send Notification:");
                print_help_box(help_text);
                printf("");
-               printf("\n", (send_notification == TRUE) ? "CHECKED" : "");
+               /* http://iamcam.wordpress.com/2008/01/15/unchecked-checkbox-values/ */
+               printf("", (send_notification == TRUE) ? 0 : 1); /* negate value hack, if checkbox is unchecked */
+               printf("\n", (send_notification == TRUE) ? 1 : 0, (send_notification == TRUE) ? "CHECKED" : "");
                break;

        case PRINT_PERSISTENT:

[1351344362] EXTERNAL COMMAND: icingademo;1.2.3.4;REMOVE_SVC_ACKNOWLEDGEMENT;1300localhost;DISEÑOS
[1351344420] EXTERNAL COMMAND: icingademo;1.2.3.4;ACKNOWLEDGE_SVC_PROBLEM;1300localhost;DISEÑOS;2;0;0;icingademo;test_no_notif
[1351344450] EXTERNAL COMMAND: icingademo;1.2.3.4;REMOVE_SVC_ACKNOWLEDGEMENT;1300localhost;DISEÑOS
[1351344469] EXTERNAL COMMAND: icingademo;1.2.3.4;ACKNOWLEDGE_SVC_PROBLEM;1300localhost;DISEÑOS;1;1;0;icingademo;test_no_sticky
[1351344494] EXTERNAL COMMAND: icingademo;1.2.3.4;REMOVE_SVC_ACKNOWLEDGEMENT;1300localhost;DISEÑOS
[1351344523] EXTERNAL COMMAND: icingademo;1.2.3.4;ACKNOWLEDGE_SVC_PROBLEM;1300localhost;DISEÑOS;2;1;0;icingademo;test_both_enabled
[1351344543] EXTERNAL COMMAND: icingademo;1.2.3.4;REMOVE_SVC_ACKNOWLEDGEMENT;1300localhost;DISEÑOS
[1351344569] EXTERNAL COMMAND: icingademo;1.2.3.4;ACKNOWLEDGE_SVC_PROBLEM;1300localhost;DISEÑOS;1;0;0;icingademo;test_both_disabled

i'm pretty sure Ricardo will change that to somewhat better, so leaving this around for now.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Nov 9, 2012

Updated by ricardo on 2012-11-09 17:02:49 +00:00

  • Subject changed from acknowledgement cmd sends negated flags for send_notification and sticky_ack to committing acknowledgement cmd negates flags for send_notification and sticky_ack
  • Status changed from Assigned to 7

fixed in current dev/cgis

please test

Found another way on checking if the default TRUE values for send_notification and sticky_ack are set or not. Don't really like the idea of adding a hidden html input and hoping that every browser behaves the same.

Cheers Ricardo

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Nov 10, 2012

Updated by mfriedrich on 2012-11-10 14:41:43 +00:00

(deleting the ack before every test case)

ticked both

Nov 10 15:35:48 sol icinga: EXTERNAL COMMAND: ACKNOWLEDGE_SVC_PROBLEM;1300localhost;DISEÑOS;2;1;0;icingademo;test1
Nov 10 15:35:48 sol icinga: SERVICE NOTIFICATION: testconfig-admin;1300localhost;DISEÑOS;ACKNOWLEDGEMENT (CRITICAL);true;CRITICAL: foobaer;icingademo;test1

ticked sticky ack, no notifications

Nov 10 15:37:27 sol icinga: EXTERNAL COMMAND: ACKNOWLEDGE_SVC_PROBLEM;1300localhost;DISEÑOS;2;0;0;icingademo;test2

ticked notifications, no sticky ack

Nov 10 15:38:29 sol icinga: EXTERNAL COMMAND: ACKNOWLEDGE_SVC_PROBLEM;1300localhost;DISEÑOS;1;1;0;icingademo;test3
Nov 10 15:38:29 sol icinga: SERVICE NOTIFICATION: testconfig-admin;1300localhost;DISEÑOS;ACKNOWLEDGEMENT (CRITICAL);true;CRITICAL: foobaer;icingademo;test3

ticked none.

Nov 10 15:39:30 sol icinga: EXTERNAL COMMAND: ACKNOWLEDGE_SVC_PROBLEM;1300localhost;DISEÑOS;1;0;0;icingademo;test4

so working as expected, thanks! (and for the ugly fix - well, there's always a workaround to html design fails...)

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Nov 10, 2012

Updated by mfriedrich on 2012-11-10 14:41:53 +00:00

  • Status changed from 7 to Resolved
  • Done % changed from 0 to 100
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Dec 8, 2014

Updated by mfriedrich on 2014-12-08 09:27:46 +00:00

  • Project changed from 19 to Core, Classic UI, IDOUtils
  • Category changed from 52 to Classic UI
  • OS Version set to any
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.