D22717: Add Date term to KActivities Stats to filter on resource event date

Ivan Čukić noreply at phabricator.kde.org
Wed Jul 24 21:02:06 BST 2019


ivan requested changes to this revision.
ivan added a comment.
This revision now requires changes to proceed.


  Joining with `ResourceEvent` is a problem. At least make it not get joined if the date filter is not present.

INLINE COMMENTS

> QueryTest.h:53
>      void testFancySyntaxOrderingDefinition();
> +    void tesDateSyntaxOrderingDefinition();
>  

tes -> test

> resultset.cpp:182
>  
> +    QString dateClause(const QDate &date) const {
> +        return date.isNull() ? QStringLiteral("1") :

setDate above is by value, here it is const-ref. Check for the size of the type and decide on one of these.

> resultset.cpp:298
>                  ON rl.targettedResource = ri.targettedResource
> +           LEFT JOIN
> +               ResourceEvent re

This is a potential efficiency problem - ResourceEvent is the quickest growing table. I don't like the idea for it to always be joined.

> resultset.cpp:416
>                          ResourceScoreCache rsc
> +                    LEFT JOIN
> +                       ResourceEvent re

Also, add an empty line above - I know I like empty lines too much, but ... :)

REPOSITORY
  R159 KActivities Statistics

REVISION DETAIL
  https://phabricator.kde.org/D22717

To: meven, ivan, #frameworks
Cc: kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190724/01c8088c/attachment.html>


More information about the Kde-frameworks-devel mailing list