[Merge] lp:~renatofilho/address-book-service/sync-first into lp:address-book-service

Michael Sheldon michael.sheldon at canonical.com
Thu Oct 15 16:51:00 UTC 2015


Review: Needs Fixing

Code looks good, just a few typos in comments, warnings, etc.

Diff comments:

> 
> === modified file 'updater/ab-update-buteo-import.cpp'
> --- updater/ab-update-buteo-import.cpp	2015-09-29 14:15:42 +0000
> +++ updater/ab-update-buteo-import.cpp	2015-10-15 16:01:53 +0000
> @@ -284,6 +276,114 @@
>  
>  }
>  
> +bool ButeoImport::checkOldAccounts()
> +{
> +    // check if the user has account with contacts and disabled sync
> +    m_disabledAccounts.clear();
> +    m_lastError = ABUpdateModule::NoError;
> +
> +    for(int i=0; i < m_accounts.size(); i++) {
> +        const AccountInfo &acc = m_accounts.at(i);
> +        if (!acc.syncEnabled && !acc.emptySource) {
> +            qDebug() << "Account need to be enabled:" << acc.accountId << acc.accountName;

need -> needs

> +            m_disabledAccounts << i;
> +        }
> +    }
> +
> +    askAboutDisabledAccounts();
> +}
> +
> +void ButeoImport::askAboutDisabledAccounts()
> +{
> +    if (m_disabledAccounts.isEmpty()) {
> +        syncOldContacts();
> +        return;
> +    }
> +    const AccountInfo &acc = m_accounts.at(m_disabledAccounts.first());
> +
> +    QMap<QString, QString> updateOptions;
> +    updateOptions.insert("enable", _("Enable Sync"));
> +    updateOptions.insert("disable", _("Keep Disabled"));
> +
> +    qDebug() << "Ask question if keep account" << acc.accountName;
> +    ABNotifyMessage *msg = new ABNotifyMessage(true, this);
> +    connect(msg, SIGNAL(questionReplied(QString)), this, SLOT(onEnableAccountsReplied(QString)));
> +    msg->askQuestion(_("Contact Sync Upgrade"),
> +                     TRANSFER_ICON,
> +                     QString(_("Google account <b>%1</b> currently has contact sync disabled.\n"
> +                               "We need to enable it to proceed with the contact sync upgrade.\n"
> +                               "If you keep it disabled, your contacts will be saved but you won't be able to sync them anymore with this account.")
> +                             ).arg(acc.accountName),
> +                     updateOptions);
> +}
> +
> +void ButeoImport::onEnableAccountsReplied(const QString &reply)
> +{
> +    AccountInfo &acc = m_accounts[m_disabledAccounts.takeFirst()];
> +    qDebug() << "Account" << acc.accountId << acc.accountName << reply;
> +    if (reply == "enable") {
> +        acc.enableSync(SYNCEVO_UOA_SERVICE_NAME);
> +    } else if (reply == "disable") {
> +        acc.removeAfterUpdate = false;
> +    }
> +
> +    askAboutDisabledAccounts();
> +}
> +
> +bool ButeoImport::syncOldContacts()
> +{
> +    //call syncevoltuion

syncevoltuion -> syncevolution

> +    if (m_syncMonitorInterface.isNull()) {
> +        m_syncMonitorInterface.reset(new QDBusInterface(SYNCMONITOR_DBUS_SERVICE_NAME,
> +                                                        SYNCMONITOR_DBUS_OBJECT_PATH,
> +                                                        SYNCMONITOR_DBUS_INTERFACE));
> +
> +        if (!m_buteoInterface->isValid()) {
> +            m_buteoInterface.reset();
> +            qWarning() << "Fail to connect with sync-monitor";
> +            return false;
> +        }
> +
> +        connect(m_syncMonitorInterface.data(), SIGNAL(syncFinished(QString, QString)),
> +                SLOT(onOldContactsSyncFinished(QString,QString)), Qt::UniqueConnection);
> +        connect(m_syncMonitorInterface.data(), SIGNAL(syncError(QString, QString, QString)),
> +                SLOT(onOldContactsSyncError(QString,QString,QString)), Qt::UniqueConnection);
> +    }
> +
> +    m_syncEvolutionQueue.clear();
> +    for(int i=0; i < m_accounts.size(); i++) {
> +        const AccountInfo &accInfo = m_accounts[i];
> +        // if the account is disabled or the new source was already created we do not need to sync
> +        if (accInfo.syncEnabled &&
> +            accInfo.newSourceId.isEmpty() &&
> +            !accInfo.oldSourceId.isEmpty()) {
> +            qDebug() << "SyncEvolution: Prepare to sync" << accInfo.accountId << accInfo.accountName;
> +            m_syncEvolutionQueue << i;
> +        } else {
> +            qDebug() << "SyncEvolution: Skip sync for disabled account" << accInfo.accountId << accInfo.accountName;
> +        }
> +    }
> +
> +    syncOldContactsContinue();
> +}
> +
> +void ButeoImport::syncOldContactsContinue()
> +{
> +    if (m_syncEvolutionQueue.isEmpty()) {
> +        continueUpdate();
> +        return;
> +    }
> +
> +    const AccountInfo &accInfo = m_accounts[m_syncEvolutionQueue.first()];
> +    QDBusReply<void> result = m_syncMonitorInterface->call("syncAccount", accInfo.accountId, "contacts");
> +    if (result.error().isValid()) {
> +        qWarning() << "SyncEvolution: Fail to start account sync" << accInfo.accountId  << accInfo.accountName << result.error();
> +        onError("", ButeoImport::InitialSyncError, true);
> +    } else {
> +        qDebug() << "SyncEvolution: Syncing" << accInfo.accountId << accInfo.accountName;
> +    }
> +}
> +
>  ABUpdateModule::ImportError ButeoImport::parseError(int errorCode) const
>  {
>      switch (errorCode)
> @@ -702,29 +912,33 @@
>      case 2:
>          return;
>      case 3:
> -        qWarning() << "Sync error for account:" << accountId  << "and profile" << profileName;
> -        m_failToSyncProfiles << profileName;
> -        m_lastError = parseError(moreDetails);
> +        if (!accInfo.syncEnabled) {
> +            // error because the accout is not enabled

accout -> account

> +        } else {
> +            qWarning() << "Sync error for account:" << accInfo.accountId  << "and profile" << profileName;
> +            m_failToSyncProfiles << profileName;
> +            m_lastError = parseError(moreDetails);
> +        }
>          break;
>      case 4:
> -        qDebug() << "Sync finished for account:" << accountId  << "and profile" << profileName;
> +        qDebug() << "Sync finished for account:" << accInfo.accountId  << "and profile" << profileName;
>          break;
>      case 5:
> -        qWarning() << "Sync aborted for account:" << accountId  << "and profile" << profileName;
> +        qWarning() << "Sync aborted for account:" << accInfo.accountId  << "and profile" << profileName;
>          m_failToSyncProfiles << profileName;
>          break;
>      }
> -
> -    if (accountId > 0) {
> -        m_pendingAccountToProfiles.remove(accountId);
> -        if (m_pendingAccountToProfiles.isEmpty()) {
> -            qDebug() << "All accounts has fineshed the sync, number of accounts that fail to sync:" << m_failToSyncProfiles.size();
> -            if (m_failToSyncProfiles.isEmpty()) {
> -                Q_EMIT updated();
> -            } else {
> -                QMetaObject::invokeMethod(this, "onError", Qt::QueuedConnection,
> -                                          Q_ARG(QString, ""), Q_ARG(int, m_lastError));
> -            }
> +    m_buteoQueue.remove(index);
> +    qDebug() << "Accounts to sync" << m_buteoQueue;
> +    if (m_buteoQueue.isEmpty()) {
> +        qDebug() << "All accounts has fineshed the sync, number of accounts that fail to sync:" << m_failToSyncProfiles;

has fineshed -> have finished

> +        if (m_failToSyncProfiles.isEmpty()) {
> +            Q_EMIT updated();
> +        } else {
> +            QMetaObject::invokeMethod(this, "onError", Qt::QueuedConnection,
> +                                      Q_ARG(QString, ""),
> +                                      Q_ARG(int, m_lastError),
> +                                      Q_ARG(bool, true));
>          }
>      }
>  }
> 
> === modified file 'updater/ab-update.cpp'
> --- updater/ab-update.cpp	2015-09-29 14:37:03 +0000
> +++ updater/ab-update.cpp	2015-10-15 16:01:53 +0000
> @@ -118,6 +116,11 @@
>          return;
>      }
>  
> +    if (m_waitingForIntenert) {

This variable should probably be called m_waitingForInternet

> +        m_waitingForIntenert = false;
> +        m_netManager->disconnect(this);
> +    }
> +
>      m_lock.lock();
>      m_modulesToUpdate = modulesToUpdate;
>      qDebug() << "Modules to update" << m_modulesToUpdate.size();


-- 
https://code.launchpad.net/~renatofilho/address-book-service/sync-first/+merge/273885
Your team Ubuntu Phablet Team is subscribed to branch lp:address-book-service.



More information about the Ubuntu-reviews mailing list