linagora / jmap-client-ts

JMAP 1.0 client library in TypeScript
MIT License
39 stars 16 forks source link

getFirstAccountId need to be implemented as getPersonalAccountId #67

Open qyb opened 2 years ago

qyb commented 2 years ago

The spec does not guarantee the personal(primary mail) account is the first element of session.accounts. I'm not sure whether JamesServer supports account/mailbox sharing. But cyrus-imapd seems that the personal account always is the last element of session.accounts.

I suggest that we add a getPersonalAccountId() method, and keep getFirstAccountId until 2.0 release.

alagane commented 2 years ago

I suggest that we add a getPersonalAccountId() method, and keep getFirstAccountId until 2.0 release.

But isn't there the same problem with personal accounts? I am not sure I understood the specs correctly, but there might be multiple personal accounts and multiple primary accounts? @chibenwa

chibenwa commented 2 years ago

Actually:

qyb commented 2 years ago

Actually: IMO we can easily improve things by having: getPrimaryAccountId(Capability) which would mirror directly the session primaryAccounts field. IMO knowing if this is personal or not is secondary.

maybe something like (untested):

  public getPrimaryAccountId(capability: string = 'urn:ietf:params:jmap:mail'): string {
    const primaryAccounts = this.session?.primaryAccounts;
    if (primaryAccounts && capability in primaryAccounts) {
        return primaryAccounts[capability];
    }
    throw new Error('No account available for this session');
  }

is it right?

chibenwa commented 2 years ago

Maybe refining a bit the error message but yes ;-)

'No primary account available for $capability in this session' would make more sense.

Also coming from the Java world I would likely encode the 'missing' concept in the return type instead of hard coding failure, which would then allow the caller to decide what to do. Optional<String> in java. string? in typescript maybe? (I'm a typescript noob....)

qyb commented 2 years ago

Also coming from the Java world I would likely encode the 'missing' concept in the return type instead of hard coding failure, which would then allow the caller to decide what to do. Optional<String> in java. string? in typescript maybe? (I'm a typescript noob....)

Because we will use it in replaceAccountId 😅 ,

  private replaceAccountId<U extends IReplaceableAccountId>(input: U): U {
    return input.accountId !== null
      ? input
      : {
          ...input,
          accountId: this.getFirstAccountId(),
        };
  }
qyb commented 2 years ago

Because we will use it in replaceAccountId 😅 ,

  private replaceAccountId<U extends IReplaceableAccountId>(input: U): U {
    return input.accountId !== null
      ? input
      : {
          ...input,
          accountId: this.getFirstAccountId(),
        };
  }

I found that emailSubmission_get/emailSubmission_changes/emailSubmission_set should depends getPrimaryAccountId('urn:ietf:params:jmap:submission') ...

😭