Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getFirstAccountId need to be implemented as getPersonalAccountId #67

Open
qyb opened this issue Jun 26, 2022 · 6 comments
Open

getFirstAccountId need to be implemented as getPersonalAccountId #67

qyb opened this issue Jun 26, 2022 · 6 comments

Comments

@qyb
Copy link
Contributor

qyb commented Jun 26, 2022

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.

qyb added a commit to qyb/jmap-client-ts that referenced this issue Jun 27, 2022
@alagane
Copy link
Member

alagane commented Jun 27, 2022

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
Copy link
Member

chibenwa commented Jun 28, 2022

Actually:

  • Each capability have it's primary ccount

This is usefull for implementing a Proxy, where capabilities are implemented by different servers.

  • Each account can be personnal, or not.

I would expect primary accounts to be personnal, but that is already an assumption.

I suggest that we add a getPersonalAccountId() method,

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.

and keep getFirstAccountId until 2.0 release.

Is there a way to mark things as deprecated in typescript ?

@qyb
Copy link
Contributor Author

qyb commented Jun 28, 2022

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
Copy link
Member

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
Copy link
Contributor Author

qyb commented Jun 28, 2022

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
Copy link
Contributor Author

qyb commented Jun 28, 2022

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') ...

😭

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants