Merge lp:~mr-fiorotto/wingpanel-indicator-notifications/trying-to-fix-1487291 into lp:~wingpanel-devs/wingpanel-indicator-notifications/wingpanel-indicator-notifications

[Giuliano Fiorotto]

Don't save "transient" notifications

Check if the transient parameter is in the hints Variant, if true don't add the notification to the list.

[Adam Bieńkowski (donadigo)]

You should be using that check in get_valid method in Notification class instead of using it in add_entry.

review: Needs Fixing (code)

[lp:~mr-fiorotto/wingpanel-indicator-notifications/trying-to-fix-1487291]

135. By Giuliano Fiorotto on 2017-01-19

changed check position to get_is_valid #1487291

[Giuliano Fiorotto (mr-fiorotto)]

> You should be using that check in get_valid method in Notification class
> instead of using it in add_entry.

I've moved that check in get_is_valid method in Notification class.

[Adam Bieńkowski (donadigo)]

Though, this code works, you are interpreting the transient value wrong. Currently you are checking if it only *exists*, while you should be checking if the "transient" value is false.

What I mean is lookup returns if the key exists not it's actual value e.g: you can execute a "Notify" method in d-feet to see how the indicator behaves with these parameters:

"AppName", 0, "dialog-information", "Summary", "Body", {}, {"transient": GLib.Variant('b', False) }, -1

You can see even if we sent "False" your solution will mark the notification invalid.

Please use "lookup_value" and "get_boolean":
var transient = hints.lookup_value ("transient", VariantType.BOOLEAN);
return ... && (transient == null || !transient.get_boolean ());

review: Needs Fixing (code / testing)

[lp:~mr-fiorotto/wingpanel-indicator-notifications/trying-to-fix-1487291]

136. By Giuliano Fiorotto on 2017-02-05

fixed code accordig to @donadigo suggestion

[Giuliano Fiorotto (mr-fiorotto)]

We just checked if the transient field exist because in our tests if there was it was true, but of course it was wrong. Thanks for the d-feet suggestion :D

[Adam Bieńkowski (donadigo)]

Thanks for your work!

Looks good now.

review: Approve (code / testing)

=== modified file 'src/Services/Notification.vala'
--- src/Services/Notification.vala	2016-12-18 14:06:35 +0000
+++ src/Services/Notification.vala	2017-02-05 18:32:07 +0000
@@ -127,7 +127,8 @@
     }
 
     public bool get_is_valid () {
-        return app_info != null && hints.lookup_value (X_CANONICAL_PRIVATE_KEY, null) == null;
+        var transient = hints.lookup_value("transient", VariantType.BOOLEAN);
+        return app_info != null && hints.lookup_value (X_CANONICAL_PRIVATE_KEY, null) == null && (transient == null || !transient.get_boolean ());
     }
 
     public void close () {