Review Request 109825: Display KNotification and KStatusNotifier for Facebook notifications

[Kevin Krammer]

> On April 5, 2013, 4:48 p.m., Kevin Krammer wrote:
> > resources/facebook/facebookresource_notifications.cpp, line 215
> > <http://git.reviewboard.kde.org/r/109825/diff/2/?file=131464#file131464line215>
> >
> >     Actually it might even be possible to access the KComponentData object of the resource directly, KGlobal::mainComponent() looks like it would deliver that
> 
> Martin Klapetek wrote:
>     The problem in this case is that KGlobal::mainComponent() has "akonadi_facebook_resource_N" where N changes with every new resource user adds, which effectively breaks notifications. I'm not sure how to solve this properly, ideas?
> 
> Kevin Krammer wrote:
>     Can you describe how this breaks notifications?
> 
> Martin Klapetek wrote:
>     They are never displayed (the mainComponent() is used by default if nothing is passed I think). It's because it uses the component data to find the notifications config (the .notifyrc file) and if it has different name than what the component tells (and the component always has the name with the appended number), it won't load the config and won't show the notifications.
>     
>     At least that's how I understand it ;)
> 
> Kevin Krammer wrote:
>     Ah, I see. So I guess the question is if all Facebook accounts should have the same notification behavior or potentially different ones.
> 
> Martin Klapetek wrote:
>     Good question. A thing to consider - how many (normal) users have more than one active facebook account? Also there is a setting added in the config dialog to turn the notifications off, this works per account/resource basis.
>     
>     So can I propose to let this have one global notification config for now and watch for user feedback after it's merged. If there will be big enough demand for having per account notifications config, I'm happy to work on it. Otherwise I don't think it's worth it.

Agreed


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109825/#review30479
-----------------------------------------------------------


On April 5, 2013, 4:01 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109825/
> -----------------------------------------------------------
> 
> (Updated April 5, 2013, 4:01 p.m.)
> 
> 
> Review request for KDEPIM and Plasma.
> 
> 
> Description
> -------
> 
> This patch displays a KNotification whenever an unread notification exists on Facebook. This KNotification groups at most 3 notifications into one popup and then say "...and N more" if there is more. It also keeps track of which notifications were already displayed and does not display them again unless they were updated on the server. This is all stored in a separate config file. 
> 
> Then it creates a KSNI for the notifications where it always show the newest three notifications in the tooltip (regardless if it was displayed before or not) and creates a menu with the notifications, which opens browser with the notification link.
> 
> 
> Diffs
> -----
> 
>   resources/facebook/CMakeLists.txt e8c6381 
>   resources/facebook/facebookresource.h 4a16c0c 
>   resources/facebook/facebookresource.cpp 67e8f3b 
>   resources/facebook/facebookresource_notifications.cpp 7f6b8c4 
>   resources/facebook/serializer/akonadi_serializer_socialnotification.cpp a261e14 
>   resources/facebook/settingsbase.kcfg 9f8e4b5 
>   resources/facebook/settingsdialog.cpp bfb7826 
>   resources/facebook/settingsdialog.ui 68b6a24 
> 
> Diff: http://git.reviewboard.kde.org/r/109825/diff/
> 
> 
> Testing
> -------
> 
> Yes.
> 
> 
> File Attachments
> ----------------
> 
> KSNI
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/04/02/facebook_notifications.png
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130408/cd09286f/attachment-0001.html>