{"action":"created","issue":{"url":"https://api.github.com/repos/johnnyhowey/liferay-portal/issues/88","labels_url":"https://api.github.com/repos/johnnyhowey/liferay-portal/issues/88/labels{/name}","comments_url":"https://api.github.com/repos/johnnyhowey/liferay-portal/issues/88/comments","events_url":"https://api.github.com/repos/johnnyhowey/liferay-portal/issues/88/events","html_url":"https://github.com/johnnyhowey/liferay-portal/pull/88","id":81437665,"number":88,"title":"LPS-55893 Missing Notifications when user is logged out","user":{"login":"IstvanD","id":6039187,"avatar_url":"https://avatars.githubusercontent.com/u/6039187?v=3","gravatar_id":"","url":"https://api.github.com/users/IstvanD","html_url":"https://github.com/IstvanD","followers_url":"https://api.github.com/users/IstvanD/followers","following_url":"https://api.github.com/users/IstvanD/following{/other_user}","gists_url":"https://api.github.com/users/IstvanD/gists{/gist_id}","starred_url":"https://api.github.com/users/IstvanD/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/IstvanD/subscriptions","organizations_url":"https://api.github.com/users/IstvanD/orgs","repos_url":"https://api.github.com/users/IstvanD/repos","events_url":"https://api.github.com/users/IstvanD/events{/privacy}","received_events_url":"https://api.github.com/users/IstvanD/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-05-27T13:11:22Z","updated_at":"2015-06-03T07:43:52Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/johnnyhowey/liferay-portal/pulls/88","html_url":"https://github.com/johnnyhowey/liferay-portal/pull/88","diff_url":"https://github.com/johnnyhowey/liferay-portal/pull/88.diff","patch_url":"https://github.com/johnnyhowey/liferay-portal/pull/88.patch"},"body":"Hey @johnnyhowey \r\n\r\nI have a question about an issue regarding notifications.\r\n\r\nIf a user receives a notification while he is not logged in, his notifications will be deleted upon logging in.\r\nThe method ChannelImpl.isRemoveNotificationEvent() is responsible for determining which notifications are invalid and thus eligible to deletion.\r\nHowever the condition (notificationEvent.getDeliverBy() <= currentTime) doesn't really makes sense, because it is comparing a userId to a timeStamp, where the former will always be smaller than the latter.\r\n\r\nAn idea would be to change the condition to (notificationEvent.getTimestamp() >= currentTime) (like in this pull), so we would check if the sending time is later than the current time. This of course also doesn't makes sense, but at least in this case the notification could be considered invalid.\r\n\r\nAnother approach could be to check if the notification is already delivered. Unfortunately, NotificationEvent hasn't got a isDelivered() method, but UserNotificationEvent has, so we have to find the UserNotificationEvent which corresponds to the NotificationEvent.\r\n\r\nUnfortunately, this also would be problematic, because delivered events would only be deleted if the user receives the event while logged out, not when he is already logged in.\r\n\r\nThe reason is the following:\r\n\r\nWhen the user logs in, doInit() -> isRemoveNotificationEvent() gets called, so the unread events won't be marked for deletion.\r\ndoGetNotificationEvents() -> isRemoveNotificationEvent() is called periodically by the poller, which will remove all unread and unconfirmed Notification Events. The unconfirmed Notification Events are residing in the _unconfirmedNotificationEvents map, which is initialized when the user logs in. isRemoveNotificationEvent() will be called upon the items in _unconfirmedNotificationEvents. \r\n\r\nHowever, when the user receives a notification event while he is already logged in, _unconfirmedNotificationEvents will be empty, so no events will be deleted, read or unread.\r\n\r\nMy question is, in your opinion, which notifications can be considered invalid, if any, and in which cases should they be removed?\r\n\r\nYour answer would be much appreciated.\r\n\r\nThank you in advance.\r\n\r\nBest regards,\r\nIstván"},"comment":{"url":"https://api.github.com/repos/johnnyhowey/liferay-portal/issues/comments/108233536","html_url":"https://github.com/johnnyhowey/liferay-portal/pull/88#issuecomment-108233536","issue_url":"https://api.github.com/repos/johnnyhowey/liferay-portal/issues/88","id":108233536,"user":{"login":"IstvanD","id":6039187,"avatar_url":"https://avatars.githubusercontent.com/u/6039187?v=3","gravatar_id":"","url":"https://api.github.com/users/IstvanD","html_url":"https://github.com/IstvanD","followers_url":"https://api.github.com/users/IstvanD/followers","following_url":"https://api.github.com/users/IstvanD/following{/other_user}","gists_url":"https://api.github.com/users/IstvanD/gists{/gist_id}","starred_url":"https://api.github.com/users/IstvanD/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/IstvanD/subscriptions","organizations_url":"https://api.github.com/users/IstvanD/orgs","repos_url":"https://api.github.com/users/IstvanD/repos","events_url":"https://api.github.com/users/IstvanD/events{/privacy}","received_events_url":"https://api.github.com/users/IstvanD/received_events","type":"User","site_admin":false},"created_at":"2015-06-03T07:43:52Z","updated_at":"2015-06-03T07:43:52Z","body":"Hey Jon,\r\n\r\nI'm sorry, for sending such a long description. :)\r\n\r\nI'm trying to explain it shorter.\r\n\r\nBasically, this issue is not about the sending process of notifications, but about a regular polling of the event, and the corresponding decision whether a notification can be considered valid or not.\r\n\r\nWhen logging in, ChannelImpl.doInit() gets called, and then periodically ChannelImpl.doGetNotificationEvents(). In both methods, isRemoveNotificationEvent() gets called upon each notification to decide whether they should be deleted or not. \r\n\r\nThe problem is that the clause\r\n\r\n\t\tif ((notificationEvent.getDeliverBy() != 0) &&\r\n\t\t\t(notificationEvent.getDeliverBy() <= currentTime)) {\r\n\r\n\t\t\treturn true;\r\n\r\ndoesn't really makes sense, since the userId returned by getDeliveryBy() will always be \"smaller\" then currentTime.\r\n\r\nThis is a very old code, and it hasn't been touched since 4 years.\r\n"}}