[Merge] lp:~phablet-team/history-service/update-groups-to-room into lp:history-service/staging

[Gustavo Pichorim Boiko]

Review: Needs Fixing

Some things to fix.

Diff comments:

> === modified file 'plugins/sqlite/sqlitedatabase.cpp'
> --- plugins/sqlite/sqlitedatabase.cpp	2015-10-06 12:54:04 +0000
> +++ plugins/sqlite/sqlitedatabase.cpp	2016-11-08 16:05:25 +0000
> @@ -256,6 +257,22 @@
>                  return false;
>              }
>          }
> +        if (existingVersion < 13 && upgradeToVersion >= 13) {

I don't think we need to check the upgradeToVersion value here. If the user has this code, he will certainly have his database upgraded to the latest.

> +            // convert all groups to Room type depending if the mms option is enabled

Can you also add a comment here mentioning that this is specific to ofono/ofono accounts?

> +            QVariant mmsGroupChatEnabled = History::Utils::getUserValue("com.ubuntu.touch.AccountsService.Phone", "MmsGroupChatEnabled");
> +            // we must not fail if we cannot reach accounts service.
> +            if (mmsGroupChatEnabled.isValid()) {
> +                // if mms is disabled all chats will be turned into broadcast, otherwise
> +                // we turn them into Room
> +                if (mmsGroupChatEnabled.toBool()) {
> +                    if (!convertGroupChatToRoom()) {
> +                        qCritical() << "Failed to update existing group chats to Room type.";
> +                        rollbackTransaction();
> +                        return false;
> +                    }
> +                }
> +            }
> +        }
>      }
>  
>      finishTransaction();
> @@ -392,3 +409,40 @@
>  
>      return true;
>  }
> +
> +bool SQLiteDatabase::convertGroupChatToRoom()

Can you rename this function to convertOfonoGroupChatToRoom() or something like that so that it is clear which groups we are updating?

> +{
> +    QSqlQuery query(database());
> +    QString queryText = "UPDATE threads SET chatType=2 WHERE (SELECT COUNT(participantId) from thread_participants WHERE thread_participants.threadId=threads.threadId and thread_participants.accountId=threads.accountId AND thread_participants.type=threads.type) > 1";

I think we need to filter for accountId here, otherwise we risk updating chats from other CMs/protocols than just the ofono/ofono ones.

> +
> +    query.prepare(queryText);
> +    if (!query.exec()) {
> +        qWarning() << "Failed to update group chats to Room 1:" << query.executedQuery() << query.lastError();
> +        return false;
> +    }
> +    query.clear();
> +
> +    // now insert a row in chat_room_info for each room
> +    if (!query.exec("SELECT accountId, threadId from threads WHERE chatType=2")) {

Same here, I think we should only select the subset of accounts.

> +        qWarning() << "Failed to update group chats to Room 2:" << query.executedQuery() << query.lastError();
> +        return false;
> +    }
> +
> +    while (query.next()) {
> +        QSqlQuery queryInsertRoom(database());
> +        QString accountId = query.value(0).toString();
> +        QString threadId = query.value(1).toString();
> +        queryInsertRoom.prepare("INSERT INTO chat_room_info (accountId, threadId, type, joined) VALUES (:accountId,:threadId,:type,:joined)");
> +        queryInsertRoom.bindValue(":accountId", accountId);
> +        queryInsertRoom.bindValue(":threadId", threadId);
> +        queryInsertRoom.bindValue(":type", History::EventTypeText);
> +        queryInsertRoom.bindValue(":joined", true);
> +        if(!queryInsertRoom.exec()) {
> +            qWarning() << "Failed to update group chats to Room 3:" << queryInsertRoom.executedQuery() << queryInsertRoom.lastError();
> +            return false;
> +        }
> +        queryInsertRoom.clear();
> +    }
> +    query.clear();
> +}
> +


-- 
https://code.launchpad.net/~phablet-team/history-service/update-groups-to-room/+merge/310322
Your team Ubuntu Phablet Team is subscribed to branch lp:history-service/staging.