Update based on the feedback in Icinga#4956 and Icinga#4818

Author: TheFlyingCorpse

doc/12-icinga2-api.md

@@ -901,13 +901,14 @@ are disabled.
 
 Send a `POST` request to the URL endpoint `/v1/actions/acknowledge-problem`.
 
-  Parameter | Type      | Description
-  ----------|-----------|--------------
-  author    | string    | **Required.** Name of the author, may be empty.
-  comment   | string    | **Required.** Comment text, may be empty.
-  expiry    | timestamp | **Optional.** If set, the acknowledgement will vanish after this timestamp.
-  sticky    | boolean   | **Optional.** If `true`, the default, the acknowledgement will remain until the service or host fully recovers.
-  notify    | boolean   | **Optional.** If `true`, a notification will be sent out to contacts to indicate this problem has been acknowledged. The default is false.
+  Parameter  | Type      | Description
+  -----------|-----------|--------------
+  author     | string    | **Required.** Name of the author, may be empty.
+  comment    | string    | **Required.** Comment text, may be empty.
+  expiry     | timestamp | **Optional.** If set, the acknowledgement will vanish after this timestamp.
+  sticky     | boolean   | **Optional.** If `true`, the default, the acknowledgement will remain until the service or host fully recovers.
+  notify     | boolean   | **Optional.** If `true`, a notification will be sent out to contacts to indicate this problem has been acknowledged. The default is false.
+  persistent | boolean   | **Optional.** If `true`, the comment will remain after the acknowledgement either recovers or expires.
 
 In addition to these parameters a [filter](12-icinga2-api.md#icinga2-api-filters) must be provided. The valid types for this action are `Host` and `Service`.
 

lib/icinga/apiactions.cpp

@@ -202,7 +202,7 @@ Dictionary::Ptr ApiActions::AcknowledgeProblem(const ConfigObject::Ptr& object,
 	if (params->Contains("notify"))
 		notify = true;
 	if (params->Contains("persistent"))
-		persistent = (Convert::ToLong(HttpUtility::GetLastParameter(params, "persistent")) > 0 ? true : false );
+		persistent = HttpUtility::GetLastParameter(params, "persistent");
 	if (params->Contains("expiry"))
 		timestamp = HttpUtility::GetLastParameter(params, "expiry");
 	else

lib/icinga/checkable-comment.cpp

@@ -38,7 +38,11 @@ void Checkable::RemoveAllComments(void)
 void Checkable::RemoveCommentsByType(int type)
 {
 	for (const Comment::Ptr& comment : GetComments()) {
-		if (comment->GetEntryType() == type && comment->GetPersistent() == false)
+		/* Do not remove comments set as persistent when it originates from an Acknowledgement */
+		if (comment->GetEntryType() == CommentAcknowledgement && comment->GetPersistent())
+			continue;
+
+		if (comment->GetEntryType() == type)
 			Comment::RemoveComment(comment->GetName());
 	}
 }

lib/icinga/comment.cpp

@@ -248,8 +248,12 @@ void Comment::CommentsExpireTimerHandler(void)
 	}
 
 	for (const Comment::Ptr& comment : comments) {
+		/* Do not remove comments set as persistent when it originates from an Acknowledgement */
+		if (comment->GetEntryType() == CommentAcknowledgement && comment->IsActive() && comment->IsExpired() && comment->GetPersistent())
+			continue;
+
 		/* Only remove comment which are activated after daemon start. */
-		if (comment->IsActive() && comment->IsExpired() && comment->GetPersistent() == false)
+		if (comment->IsActive() && comment->IsExpired())
 			RemoveComment(comment->GetName());
 	}
 }

lib/icinga/comment.ti

@@ -87,7 +87,7 @@ class Comment : ConfigObject < CommentNameComposer
 	};
 	[config, required] String author;
 	[config, required] String text;
-	[config] int persistent;
+	[config] bool persistent;
 	[config] Timestamp expire_time;
 	[state] int legacy_id;
 };