D28957: Migrator: implement Google Resource migrator

[Igor Poboiko]

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


  I've tested it a bit too, on couple of Google accounts.
  Apart from couple nitpicks, I think it's good to go.

INLINE COMMENTS

> CMakeLists.txt:27
> +    KF5::Wallet
> +    KPim::GAPICore # dependency of Google Settings
> +    migrationshared

Is it really needed? It seems like the code is using just the DBus interface

> googleresourcemigrator.cpp:32
> +
> +#include <KWallet/kwallet.h>
> +#include <QSettings>

Duplicated include

> googleresourcemigrator.cpp:78
> +    const auto configPath = QStandardPaths::locate(QStandardPaths::ConfigLocation, configFile);
> +    return std::unique_ptr<QSettings>{new QSettings{configPath, QSettings::IniFormat}};
> +}

What would happen here if only one of the resources was present (e.g. only contacts)? Could `instance` be invalid here?

> googleresourcemigrator.cpp:202
> +        // Just to be sure, check that there are no left-over legacy instances
> +        removeLegacyInstances(instances);
> +

Although this should not happen, it will probably remove tokens from Wallet (as the result of cleanup procedure for legacy instances). As they store tokens in the same place, it will affect the new instance too. 
If so, we would like to have `backupKWalletData / restoreKWalletData` here too.

> googleresourcemigrator.cpp:282
> +    resourceSettings.setEnableIntervalCheck(calendarSettings->value(QStringLiteral("EnableIntervalCheck"), false).toBool());
> +    resourceSettings.setIntervalCheckTime(calendarSettings->value(QStringLiteral("IntervalCheckTime"), 60).toInt());
> +

Interval checking options could in principle also come from contacts resource. We can probably merge somewhat it like that:

  const auto contactSettings = settingsForResource(oldInstances.contactResource);
  [...]
  resourceSettings.setEnableIntervalCheck(calendarSettings->value(QStringLiteral("EnableIntervalCheck"), false) || contactSettings->value(QStringLiteral("EnableIntervalCheck"), false));
  resourceSettings.setIntervalCheckTime(qMin(calendarSettings->value(QStringLiteral("IntervalCheckTime"), 60).toInt(), contactSettings->value(QStringLiteral("IntervalCheckTime"), 60).toInt()));

REPOSITORY
  R44 KDE PIM Runtime

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

To: dvratil, poboiko, vkrause
Cc: kde-pim, fbampaloukas, dcaliste, dvasin, rodsevich, winterz, vkrause, mlaurent, knauss, dvratil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20200419/3554a2ca/attachment.html>