Skip to content
This repository has been archived by the owner on Jan 15, 2019. 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

@icinga-migration
Copy link

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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.
Projects
None yet
Development

No branches or pull requests

1 participant