ActiveSync: Detect user calendars and addressbooks again #951
Conversation
Regression from 535dad4
| if (!isMainAddressbook) { | ||
| addressbook.name = sanitize.nonemptylabel(change.DisplayName, addressbook.name); | ||
| } | ||
| await addressbook.save(); |
There was a problem hiding this comment.
Why do we need to save the addressbook on every login?
Calendars and address books should be saved in the DB, including their name, and re-loaded on every startup. They should not need to be created, not need to be saved (unless something actually changed), and not need to be added to appGlobal.addressbooks, because it's already in there since load, long before we get there.
There was a problem hiding this comment.
We don't; this only happens whenever FolderSync tells us about an address book. However, you're right about adding to appGlobal.addressbooks; I forgot to move that line back into the if (!addressbook) block.
| if (!isMainCalendar) { | ||
| calendar.name = sanitize.nonemptylabel(change.DisplayName, calendar.name); | ||
| } | ||
| calendar.url = sanitize.url(folderURL); |
There was a problem hiding this comment.
.url should be set. At least as long as long as they are accounts, until #778 lands.
There was a problem hiding this comment.
initFromMainAccount already sets the URL.
| appGlobal.calendars.add(calendar); | ||
| } | ||
| if (!isMainCalendar) { | ||
| calendar.name = sanitize.nonemptylabel(change.DisplayName, calendar.name); |
There was a problem hiding this comment.
Why set this on every login? The user will want to change this. You'd overwrite user changes this way.
There was a problem hiding this comment.
Again, this doesn't happen on every login, only when ActiveSync notifies us about it.
That's strange. I tested it and it did add address books and calendars for me. Did you try with a new account, or an existing one? Please try with a new account. |
NeilRashbrook
left a comment
NeilRashbrook
left a comment
There was a problem hiding this comment.
prevented ActiveSync from detecting any user calendars or addressbooks.
That's strange. I tested it and it did add address books and calendars for me.
Did you try with a new account, or an existing one?
Trying with an existing account is pointless because the code doesn't run in this case.
Please try with a new account.
Without the patch only one of the calendars was added. (I didn't check address books because there was only one to add.)
| if (!isMainAddressbook) { | ||
| addressbook.name = sanitize.nonemptylabel(change.DisplayName, addressbook.name); | ||
| } | ||
| await addressbook.save(); |
There was a problem hiding this comment.
We don't; this only happens whenever FolderSync tells us about an address book. However, you're right about adding to appGlobal.addressbooks; I forgot to move that line back into the if (!addressbook) block.
| if (!isMainCalendar) { | ||
| calendar.name = sanitize.nonemptylabel(change.DisplayName, calendar.name); | ||
| } | ||
| calendar.url = sanitize.url(folderURL); |
There was a problem hiding this comment.
initFromMainAccount already sets the URL.
| appGlobal.calendars.add(calendar); | ||
| } | ||
| if (!isMainCalendar) { | ||
| calendar.name = sanitize.nonemptylabel(change.DisplayName, calendar.name); |
There was a problem hiding this comment.
Again, this doesn't happen on every login, only when ActiveSync notifies us about it.
Regression from 535dad4 which prevented ActiveSync from detecting any user calendars or addressbooks.
This PR restores the previous behaviour, which is:
Additionally I removed all the remaining references to URLs which was the way that user calendars and addressbooks were detected before the server ID was added to the config JSON.