[Kde-pim] Review Request: Filter outgoing message before they are sent

[Thomas McGuire]

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

Ship it!


Looks good, please commit.
As usual, I've made some minor comments below, please take care of those.

Many thanks for the patch and sorry for the delay.

Please close bug 48938 with the BUG: keyword then.

Also, please CCMAIL exit3219 at gmail dot com, because he is the guy who replaced kmsender with the akonadi-based equivalent in the akonadi-ports branch, and we need a way for the filters there as well.


/trunk/KDE/kdepim/kmail/kmfilterdlg.cpp
<http://reviewboard.kde.org/r/1045/#comment1124>

    Maybe a tooltips for this and the checkbox below would be nice. The tooltips should explain in more details the options, so that the user sees the difference (there was some confusion of the "apply on sent messages" filter in some bug reports, that it didn't filter things _before_ sending)
    
    Also, the tooltip should explain that not all actions work for the "before sending" filter, like the "add tag" or "mark as read" actions.



/trunk/KDE/kdepim/kmail/kmsender.cpp
<http://reviewboard.kde.org/r/1045/#comment1125>

    Better use kWarning for error messages that should appear on the console



/trunk/KDE/kdepim/kmail/kmsender.cpp
<http://reviewboard.kde.org/r/1045/#comment1126>

    Please use the KDEPIM coding style, in particular spaces inside of parenthesis (yes, some other parts of KMail don't do that, but new code should)



/trunk/KDE/kdepim/kmail/kmsender.cpp
<http://reviewboard.kde.org/r/1045/#comment1127>

    "During" should be capitalized, as it occurs after a ':'.
    However, the complete message should be nicer, it is missing a verb for example.


- Thomas


On 2009-07-17 08:19:08, Bruno Bigras wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1045/
> -----------------------------------------------------------
> 
> (Updated 2009-07-17 08:19:08)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> This is the patch from Christian Schaarschmidt with nearly no modification. It add support to filter outgoing message before they are sent.
> 
> Should I prevent the user from being able to check "Apply this filter to sent messages" and "Apply this filter before sending messages" at the same time?
> 
> I'm not sure what Christian meant with this todo and the note with the commit on the bug report. I was able to add an header for a encrypted email but I'm not sure it didn't screw up something.
> 
> kmsender.cpp#504
> " // TODO: to support encrypted/signed messages this sould be moved to messagecomposer.cpp
>   // Disable the emitting of msgAdded signal, because the message is taken out of the
>   // current folder (outbox) and re-added, to make filter actions changing the message
>   // work. We don't want that to screw up message counts."
> 
> "Note: Quick win. Works for headers and clear text mails."
> https://bugs.kde.org/show_bug.cgi?id=48938#c15
> 
> 
> This addresses bug 48938.
>     https://bugs.kde.org/show_bug.cgi?id=48938
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/kmail/kmfilter.h 998013 
>   /trunk/KDE/kdepim/kmail/kmfilter.cpp 998013 
>   /trunk/KDE/kdepim/kmail/kmfilterdlg.h 998013 
>   /trunk/KDE/kdepim/kmail/kmfilterdlg.cpp 998013 
>   /trunk/KDE/kdepim/kmail/kmfiltermgr.h 998013 
>   /trunk/KDE/kdepim/kmail/kmfiltermgr.cpp 998013 
>   /trunk/KDE/kdepim/kmail/kmsender.cpp 998013 
> 
> Diff: http://reviewboard.kde.org/r/1045/diff
> 
> 
> Testing
> -------
> 
> I was able to add an header to email I was sending.
> 
> 
> Screenshots
> -----------
> 
> 
>   http://reviewboard.kde.org/r/1045/s/149/
> 
> 
> Thanks,
> 
> Bruno
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/