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

[Riccardo Padovani]

Fixed #1392197 - Support for multiple accounts does not work

Fixed #1392197 - Support for multiple accounts does not work

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

PASSED: Continuous integration, rev:312
http://91.189.93.70:8080/job/reminders-app-ci/620/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/202
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/202/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-vivid-amd64-ci/41

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

review: Approve (continuous-integration)

[Michael Zanetti (mzanetti)]

this seems to work fine, however there still is an issue with non authorized accounts. The account selection says "accountname - Tap to authorize" as expected, but tapping it actually doesn't do anyhting.

Fine with me to address that in a separate MP though. David, your opinion?

review: Approve

[David Planella (dpm)]

I think it'd be good to fix it in this branch. However, another question:
- I remember having done the change that's being reverted here to enable using the sandbox service (which we might still be using for some tests?), as instructed by mardy. Does this new change mean that we won't be able to use the sandbox anymore?

review: Needs Information

[Michael Zanetti (mzanetti)]

@David, no this doesn't break logging in with sandbox accounts, however, thinking about it, this would list enabled sandbox accounts AND production accounts in the list. Given that we only switch between those two using a cmdline argument, I guess that's still bogus. Changing my vote to needs fixing.

Also, as David said, please try to fix the "tap to authorize"

review: Needs Fixing

[David Planella (dpm)]

Ah, that reminds me of something else I wanted to mention. Some time after the work was started on the original multiple accounts branch, we had a talk with mardy and he pointed us to this workflow for multiple accounts:

https://wiki.ubuntu.com/OnlineAccounts#App_access

Rather than the originally suggested "tap to authorize" workflow, we should probably implement the above. Perhaps it will need to be in two stages: first fix the fact that accounts are not shown in this branch, and implement the app access workflow in another.

[David Planella (dpm)]

After talking to Riccardo, we'll reject it and work on this once the offline mode has landed.

review: Disapprove

Unmerged revisions

312. By Riccardo Padovani on 2014-11-13

Fixed 1392197 - Support for multiple accounts does not work

=== modified file 'src/app/qml/reminders.qml'
--- src/app/qml/reminders.qml	2014-11-07 20:45:48 +0000
+++ src/app/qml/reminders.qml	2014-11-13 11:16:46 +0000
@@ -137,7 +137,7 @@
 
     AccountServiceModel {
         id: allAccounts
-        service: useSandbox ? "evernote-sandbox" : "evernote"
+        applicationId: "com.ubuntu.reminders_reminders"
         includeDisabled: true
     }