mustang-im / mustang

Mustang - New full-featured desktop email, chat and video conference client
https://mustang.im
Other
8 stars 1 forks source link

EWS: Contacts: Save fails with `null.callEWS()` #148

Open benbucksch opened 1 month ago

benbucksch commented 1 month ago

Reproduction:

Actual result: Error (see below)

Expected result:

Stack: image

benbucksch commented 1 month ago

Probably this.addressbook.account is not set:

let response = await this.addressbook.account.callEWS(request);
NeilRashbrook commented 1 month ago

The code currently relies on the startup process calling EWSAccount.login to complete initialisation. If you edited the contact in the 1.5 seconds that this takes, or the login failed, or you deleted the EWSAccount but left the EWSAddressbook, then the addressbook won't have completed initialisation and this error will occur. I don't know how to guard against this because the login call is the only initialisation call that any of the objects get.

benbucksch commented 1 month ago

I see. You can add an init() function to Account, if absolutely needed. Would be nice to avoid it, though.

What happens to EWSAddressbook (e.g. in SQL contacts.db) after the user deleted the mail account?

NeilRashbrook commented 1 month ago

I see. You can add an init() function to Account, if absolutely needed. Would be nice to avoid it, though.

Do you want it on all accounts, something like this, or just certain types?

// app/logic/Abstract/Account.ts
// ...
export function getAllAccounts(): Collection<Account> {
// ...
// app/logic/startup.ts
// ...
import { getAllAccounts } from './Abstract/Account.ts';
// ...
export async function getStartObjects(): Promise<void> {
  // ...
  for (let account of getAllAccounts()) {
    account.init();
  }
  // ...
}

What happens to EWSAddressbook (e.g. in SQL contacts.db) after the user deleted the mail account?

Nothing happens. Should I be overriding deleteIt()?

benbucksch commented 1 month ago

Do you want it on all accounts

If we do an init(), we'd have to do it for all accounts, yes.

Nothing happens.

You mean we're leaving the address book behind?

I think we need the concept of associated or dependent accounts. E.g. Google will also have an address book, calendar, file sharing and meeting account. A user would expect that they go away when he removes the mail account.

Would that help with your other problem above?

NeilRashbrook commented 1 month ago

I can see how that would at least take care of deleting dependent accounts, but I don't know how you would want to link the accounts in the first place. Otherwise, to take my example, I would have to add code to deleteIt to search for the dependent accounts anyway, because I couldn't guarantee that the account had been logged in originally.

benbucksch commented 1 month ago

Neil and I discussed this on the phone today. The proper fix is #155 .