[Merge] lp:~phablet-team/messaging-app/add-missing-threads-to-history-filter into lp:messaging-app/staging

Gustavo Pichorim Boiko gustavo.boiko at canonical.com
Thu Nov 17 16:53:42 UTC 2016


Review: Needs Fixing

Just a couple remarks

Diff comments:

> === modified file 'src/qml/Messages.qml'
> --- src/qml/Messages.qml	2016-11-11 13:56:22 +0000
> +++ src/qml/Messages.qml	2016-11-17 16:43:38 +0000
> @@ -1039,15 +1040,15 @@
>  
>          onMessageSent: {
>              // create the new thread and update the threadId list
> -            if (accountId != messages.account.accountId ||
> -                messages.threads.length === 0) {
> +            if (messages.threads.length === 0 ||
> +                !checkThreadInFilters(accountId, messages.threadId)) {

I think you don't need to check the threads length here, the function checkThreadInFilters already covers that.

>                  addNewThreadToFilter(accountId, properties)
>              }
>          }
>          onMessageSendingFailed: {
>              // create the new thread and update the threadId list
> -            if (accountId != messages.account.accountId ||
> -                messages.threads.length === 0) {
> +            if (messages.threads.length === 0 ||
> +                !checkThreadInFilters(accountId, messages.threadId)) {

Same here.

>                  addNewThreadToFilter(accountId, properties)
>              }
>          }


-- 
https://code.launchpad.net/~phablet-team/messaging-app/add-missing-threads-to-history-filter/+merge/311172
Your team Ubuntu Phablet Team is subscribed to branch lp:messaging-app/staging.



More information about the Ubuntu-reviews mailing list