Merge lp:~rpadovani/reminders-app/multipleAccounts into lp:reminders-app

[Riccardo Padovani]

Fixed change accounts page

Implemented new change accounts page as discussed during lasts meetings.

When you change account you don't see account name due https://bugs.launchpad.net/ubuntu-system-settings-online-accounts/+bug/1374432

For the same reason, autologin doesn't work

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:259
http://91.189.93.70:8080/job/reminders-app-ci/543/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/779
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/779/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-utopic-amd64-ci/239

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/reminders-app-ci/543/rebuild

review: Approve (continuous-integration)

[David Planella (dpm)]

Thanks Riccardo!

After testing this on the emulator, I've observed a few things:

- Nice work with the Accounts action on the toolbar.
- Switching accounts seems to freeze the app. I'm not sure if it's an issue with the app or the emulator
- I added my first account with reminders, closed the app and the second one via System settings, and it then showed me a message about the account needing authorization. Functionally, this demonstrates that it works, but the placement of the message might be a bit confusing, as it seems it refers to the account I had already authorized. I would suggest to do the following changes:

http://i.imgur.com/ujy3RQ7.jpg

In summary, have an option selector, and when the unauthorized account is tapped, then the authorization dialog should pop up, rather than using the "Add account" for two different and unrelated purposes.

review: Needs Fixing

[Riccardo Padovani (rpadovani)]

Hey David,
thanks for your test and your suggestions.

So, I implemented the page as you suggested, but I have your same problem on Unity 7: when you choose an account, it freezes the app. I set this branch WIP until I understand what's going on.

[David Planella (dpm)]

So I've tested again on the emulator: switching accounts freezes the app. On a device with rtm image 81, it won't even add any accounts (*). Not sure what's going on, as with trunk adding accounts does work.

(*) when clicking to add an account, I get a full-screen blank page.

[Riccardo Padovani (rpadovani)]

I did last modifications to UI, I'm investigating on freeze

[David Planella (dpm)]

Thanks! The branch has now conflicts, so it needs merging trunk into it.

Also, if adding an UbuntuShape complicates the code, then we can perhaps leave it out. The reason I suggested it was because I thought we could use a standard OptionSelector. Would it not be possible at all?

review: Needs Fixing

[Riccardo Padovani (rpadovani)]

On Sun, Oct 12, 2014 at 06:24:38AM -0000, David Planella wrote:
>
> Review: Needs Fixing
>
> Thanks! The branch has now conflicts, so it needs merging trunk into it.

Thanks for the notice. I merged with trunk.

> Also, if adding an UbuntuShape complicates the code, then we can perhaps leave it out. The reason I suggested it was because I thought we could use a standard OptionSelector. Would it not be possible at all?

Well, you can see the code, do you think is too much complicate?
Anyway, there is no (simple? I think all is possible...) way to use
OptionSelector right now, because we need to add a different delegate for
unknown account.
So, ListView has property footer, and we can use that, but OptionSelector has
nothing like this, you can use only one type of delegate for all things in the
model.

Or we change our approach to the unknown account (like the first I had) and we
can use OptionSelector for the regular accounts, or this is the simplest way I
found to implement UbuntuShape.

Let me know!

[David Planella (dpm)]

After the conversation on IRC, if that makes things easier for the implementation, I'd suggest this new sketch http://i.imgur.com/zT8A3lo.jpg

The point is to separate the authorized from unauthorized accounts. I'm not too worried if the unauthorized accounts can be authorized individually in a list or if there is a single button to authorize them all in system settings.

[Riccardo Padovani (rpadovani)]

Aha. I read the documentation and I found a way to implement your first mockup - only the UI for now, I'm working on the backend

[Riccardo Padovani (rpadovani)]

This is ready to be reviewed now

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:266
http://91.189.93.70:8080/job/reminders-app-ci/562/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1218
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1218/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-utopic-amd64-ci/258

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/reminders-app-ci/562/rebuild

review: Approve (continuous-integration)

[Michael Zanetti (mzanetti)]

one inline comment

[lp:~rpadovani/reminders-app/multipleAccounts]

267. By Riccardo Padovani on 2014-10-24

Better implementation of a string

[Riccardo Padovani (rpadovani)]

Done

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:267
http://91.189.93.70:8080/job/reminders-app-ci/567/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1250
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1250/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-utopic-amd64-ci/263

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/reminders-app-ci/567/rebuild

review: Approve (continuous-integration)

[Michael Zanetti (mzanetti)]

let's get this in. thanks

review: Approve

[David Planella (dpm)]

For some reason, when testing trunk I can't see any action to switch accounts.

[Michael Zanetti (mzanetti)]

Weird thing: It works fine on the desktop for me, but not on the phone. I think it's a difference in behavior of OA.

=== modified file '3rdParty/libthrift/transport/TSSLSocket.cpp'
--- 3rdParty/libthrift/transport/TSSLSocket.cpp	2013-11-21 23:30:15 +0000
+++ 3rdParty/libthrift/transport/TSSLSocket.cpp	2014-10-24 13:44:56 +0000
@@ -143,9 +143,11 @@
 void TSSLSocket::close() {
   if (ssl_ != NULL) {
     int rc = SSL_shutdown(ssl_);
-    if (rc == 0) {
-      rc = SSL_shutdown(ssl_);
-    }
+    // FIXME: this freezes the app when switching account, check if it's needed
+    // when upgrade evernote sdk
+//    if (rc == 0) {
+//      rc = SSL_shutdown(ssl_);
+//    }
     if (rc < 0) {
       int errno_copy = errno;
       string errors;
 
=== modified file 'src/app/qml/reminders.qml'
--- src/app/qml/reminders.qml	2014-10-10 14:15:04 +0000
+++ src/app/qml/reminders.qml	2014-10-24 13:44:56 +0000
@@ -57,11 +57,12 @@
     property var accountPage;
 
     function openAccountPage(isChangingAccount) {
+        var unauthorizedAccounts = allAccounts.count - accounts.count > 0 ? true : false
         if (accountPage) {
             accountPage.destroy(100)
         }
         var component = Qt.createComponent(Qt.resolvedUrl("ui/AccountSelectorPage.qml"));
-        accountPage = component.createObject(root, {accounts: accounts, isChangingAccount: isChangingAccount});
+        accountPage = component.createObject(root, { accounts: allAccounts, isChangingAccount: isChangingAccount, unauthorizedAccounts: unauthorizedAccounts });
         accountPage.accountSelected.connect(function(handle) { accountService.objectHandle = handle; pagestack.pop(); root.accountPage = null });
         pagestack.push(accountPage);
     }
@@ -134,6 +135,12 @@
         applicationId: "com.ubuntu.reminders_reminders"
     }
 
+    AccountServiceModel {
+        id: allAccounts
+        service: useSandbox ? "evernote-sandbox" : "evernote"
+        includeDisabled: true
+    }
+
     AccountService {
         id: accountService
         onObjectHandleChanged: {
 
=== modified file 'src/app/qml/ui/AccountSelectorPage.qml'
--- src/app/qml/ui/AccountSelectorPage.qml	2014-09-23 12:39:27 +0000
+++ src/app/qml/ui/AccountSelectorPage.qml	2014-10-24 13:44:56 +0000
@@ -28,8 +28,9 @@
     objectName: "Accountselectorpage"
     title: i18n.tr("Select Evernote account")
 
-    property alias accounts: listView.model
+    property alias accounts: optionSelector.model
     property bool isChangingAccount
+    property bool unauthorizedAccounts
 
     signal accountSelected(var handle)
 
@@ -41,33 +42,54 @@
 
     Column {
         anchors { fill: parent; margins: units.gu(2) }
-        spacing: units.gu(1)
+        spacing: units.gu(2)
 
-        ListView {
-            id: listView
+        OptionSelector {
+            id: optionSelector
             width: parent.width
-            height: units.gu(10)
-            model: accounts
-            currentIndex: -1
+            expanded: true
 
             delegate: Standard {
                 objectName: "EvernoteAccount"
                 text: displayName
+                showDivider: index + 1 !== accounts.count
+
                 MouseArea {
                     anchors.fill: parent
-                    onClicked: root.accountSelected(accountServiceHandle)
-                }
-            }
-
-            footer: Button {
-                text: i18n.tr("Add account")
-                onClicked: setup.exec()
-            }
-        }
-    }
-
-    tools: ToolbarItems {
-        locked: !isChangingAccount
-        opened: isChangingAccount
-    }
+                    onClicked: {
+                        if (model.enabled) {
+                            root.accountSelected(accountServiceHandle)
+                        }
+                        else {
+                            console.log('authorize')
+                        }
+                    }
+                }
+
+                Component.onCompleted: {
+                    if (isChangingAccount && displayName == preferences.accountName) {
+                        optionSelector.selectedIndex = index;
+                    }
+                    if (!model.enabled) {
+                        text = i18n.tr("%1 - Tap to authorize").arg(text)
+                    }
+                }
+            }
+        }
+
+        Button {
+            anchors.horizontalCenter: parent.horizontalCenter
+            width: parent.width - units.gu(2)
+            text: i18n.tr("Add new account")
+            color: UbuntuColors.orange
+            onClicked: setup.exec()
+        }
+     }
+
+     head.backAction: Action {
+         visible: isChangingAccount
+         iconName: "back"
+         text: i18n.tr("Back")
+         onTriggered: { pagestack.pop(); }
+     }
 }
 
=== modified file 'src/app/qml/ui/NotebooksPage.qml'
--- src/app/qml/ui/NotebooksPage.qml	2014-09-23 12:39:27 +0000
+++ src/app/qml/ui/NotebooksPage.qml	2014-10-24 13:44:56 +0000
@@ -72,7 +72,7 @@
             action: Action {
                 text: i18n.tr("Accounts")
                 iconName: "contacts-app-symbolic"
-                visible: accounts.count > 1
+                visible: allAccounts.count > 1
                 onTriggered: {
                     openAccountPage(true);
                 }
 
=== modified file 'src/app/qml/ui/NotesPage.qml'
--- src/app/qml/ui/NotesPage.qml	2014-10-20 15:02:09 +0000
+++ src/app/qml/ui/NotesPage.qml	2014-10-24 13:44:56 +0000
@@ -97,7 +97,7 @@
             action: Action {
                 text: i18n.tr("Accounts")
                 iconName: "contacts-app-symbolic"
-                visible: accounts.count > 1
+                visible: allAccounts.count > 1
                 onTriggered: {
                     openAccountPage(true);
                 }
 
=== modified file 'src/app/qml/ui/RemindersPage.qml'
--- src/app/qml/ui/RemindersPage.qml	2014-10-15 10:24:28 +0000
+++ src/app/qml/ui/RemindersPage.qml	2014-10-24 13:44:56 +0000
@@ -43,7 +43,7 @@
             action: Action {
                 text: i18n.tr("Accounts")
                 iconName: "contacts-app-symbolic"
-                visible: accounts.count > 1
+                visible: allAccounts.count > 1
                 onTriggered: {
                     openAccountPage(true);
                 }
 
=== modified file 'src/plugin/Evernote/evernoteconnection.cpp'
--- src/plugin/Evernote/evernoteconnection.cpp	2014-09-19 21:31:39 +0000
+++ src/plugin/Evernote/evernoteconnection.cpp	2014-10-24 13:44:56 +0000
@@ -169,6 +169,9 @@
         m_token = token;
         emit tokenChanged();
 
+        qDeleteAll(m_jobQueue);
+        m_jobQueue.clear();
+
         connectToEvernote();
     }
 }
 
=== modified file 'src/plugin/Evernote/notesstore.cpp'
--- src/plugin/Evernote/notesstore.cpp	2014-10-09 17:24:22 +0000
+++ src/plugin/Evernote/notesstore.cpp	2014-10-24 13:44:56 +0000
@@ -53,6 +53,7 @@
     connect(EvernoteConnection::instance(), &EvernoteConnection::isConnectedChanged, this, &NotesStore::refreshNotebooks);
     connect(EvernoteConnection::instance(), SIGNAL(isConnectedChanged()), this, SLOT(refreshNotes()));
     connect(EvernoteConnection::instance(), &EvernoteConnection::isConnectedChanged, this, &NotesStore::refreshTags);
+    connect(EvernoteConnection::instance(), &EvernoteConnection::tokenChanged, this, &NotesStore::clear);
 
     qRegisterMetaType<evernote::edam::NotesMetadataList>("evernote::edam::NotesMetadataList");
     qRegisterMetaType<evernote::edam::Note>("evernote::edam::Note");
@@ -334,13 +335,7 @@
 void NotesStore::refreshNotes(const QString &filterNotebookGuid)
 {
     if (EvernoteConnection::instance()->token().isEmpty()) {
-        beginResetModel();
-        foreach (Note *note, m_notes) {
-            emit noteRemoved(note->guid(), note->notebookGuid());
-            note->deleteLater();
-        }
-        m_notes.clear();
-        endResetModel();
+        clear();
         emit countChanged();
     } else {
         m_loading = true;
@@ -782,3 +777,15 @@
     int idx = m_notes.indexOf(note);
     emit dataChanged(index(idx), index(idx));
 }
+
+void NotesStore::clear()
+{
+    beginResetModel();
+    foreach (Note *note, m_notes) {
+        emit noteRemoved(note->guid(), note->notebookGuid());
+        note->deleteLater();
+    }
+    m_notes.clear();
+    m_notesHash.clear();
+    endResetModel();
+}
 
=== modified file 'src/plugin/Evernote/notesstore.h'
--- src/plugin/Evernote/notesstore.h	2014-10-09 00:08:52 +0000
+++ src/plugin/Evernote/notesstore.h	2014-10-24 13:44:56 +0000
@@ -162,6 +162,8 @@
     void saveTagJobDone(EvernoteConnection::ErrorCode errorCode, const QString &errorMessage);
 
     void emitDataChanged();
+    void clear();
+
 private:
     explicit NotesStore(QObject *parent = 0);
     static NotesStore *s_instance;