linagora / tmail-backend

GNU Affero General Public License v3.0
41 stars 22 forks source link

[TeamMailbox] Members/get JMAP method #1088

Closed Arsnael closed 4 months ago

Arsnael commented 5 months ago

Request:

[
    "Members/get",
    {
        "accountId": "xyz",
        "ids": ["team-mailbox-name"]
    },
    "#0
]  

Response:

[
    "Members/get",
    {
        "list": [{
            "id": "team-mailbox-name",
            "members": {
                "bob@domain.tld": {"role":"manager"},
                "alice@domain.tld": {"role":"member"}
             }
         }]
    },
    "#0
]

Notes:

DoD: Integration tests

hungphan227 commented 4 months ago

I think response should be:

[
    "Members/get",
    {
        "list": [{
            "id": "team-mailbox-name",
            "members": [
                {
                    "username": "bob@domain.tld" ,
                    "role": "manager"
                },
                {
                    "username": "alice@domain.tld" ,
                    "role": "member"
                }
             ]
         }]
    },
    "#0
]
Arsnael commented 4 months ago

@hungphan227 I'm ok with this

quantranhong1999 commented 4 months ago

I think response should be:

Why? I tend not to agree with that.

Remember that the /set update relies on the username as the key to update the role. And that is why I proposed members: String[Role]. A JSON object where the keys are usernames (as strings) and the values are Role objects. cf https://github.com/linagora/tmail-backend/blob/master/docs/modules/ROOT/pages/tmail-backend/jmap-extensions/jmapTeamMailboxMembers.adoc#teammailboxmember-object

Just like the JMAP specs for Email/set keywords: keywords: String[Boolean] (default: {}) A set of keywords that apply to the Email. The set is represented as an object, with the keys being the keywords. The value for each key in the object MUST be true. cf https://jmap.io/spec-mail.html#properties-of-the-email-object

What is the point of /set update by the key but not knowing which is the key for the patch (being hidden in the object as other fields)?

hungphan227 commented 4 months ago

What is the point of /set update by the key but not knowing which is the key for the patch (being hidden in the object as other fields)?

Is it just your idea or is it based on any standard?

quantranhong1999 commented 4 months ago

Is it just your idea or is it based on any standard?

Base on the Email/set update keywords standard, as I said above.

hungphan227 commented 4 months ago

Is it just your idea or is it based on any standard?

Base on the Email/set update keywords standard, as I said above.

I ask Email/set, not Email/get

quantranhong1999 commented 4 months ago

I ask Email/set, not Email/get

Actually, it is the Email object that is shared by both Email/set and Email/get. Feel free to read the JMAP Email specs, examples, and contract tests... to double check that :)

I am surprised that you approved this extension specs PR though.

hungphan227 commented 4 months ago

pr https://github.com/linagora/tmail-backend/pull/1111