From 1d998ec58d836d85ba6be1c8600a5a6ec7eca897 Mon Sep 17 00:00:00 2001 From: Pascal Bleser Date: Fri, 24 Oct 2025 19:22:30 +0200 Subject: [PATCH] groupware: add Mailbox sorting --- pkg/jmap/jmap_api_mailbox.go | 19 ++-- .../pkg/groupware/groupware_api_mailbox.go | 87 +++++++++++++++++-- .../groupware/pkg/groupware/groupware_test.go | 17 ++++ 3 files changed, 102 insertions(+), 21 deletions(-) diff --git a/pkg/jmap/jmap_api_mailbox.go b/pkg/jmap/jmap_api_mailbox.go index 0d2635b59..b9ee098c5 100644 --- a/pkg/jmap/jmap_api_mailbox.go +++ b/pkg/jmap/jmap_api_mailbox.go @@ -40,18 +40,13 @@ func (j *Client) GetMailbox(accountId string, session *Session, ctx context.Cont }) } -type AllMailboxesResponse struct { - Mailboxes []Mailbox `json:"mailboxes"` - State State `json:"state"` -} - -func (j *Client) GetAllMailboxes(accountIds []string, session *Session, ctx context.Context, logger *log.Logger, acceptLanguage string) (map[string]AllMailboxesResponse, SessionState, Language, Error) { +func (j *Client) GetAllMailboxes(accountIds []string, session *Session, ctx context.Context, logger *log.Logger, acceptLanguage string) (map[string]Mailboxes, SessionState, Language, Error) { logger = j.logger("GetAllMailboxes", session, logger) uniqueAccountIds := structs.Uniq(accountIds) n := len(uniqueAccountIds) if n < 1 { - return map[string]AllMailboxesResponse{}, "", "", nil + return nil, "", "", nil } invocations := make([]Invocation, n) @@ -61,19 +56,19 @@ func (j *Client) GetAllMailboxes(accountIds []string, session *Session, ctx cont cmd, err := j.request(session, logger, invocations...) if err != nil { - return map[string]AllMailboxesResponse{}, "", "", err + return nil, "", "", err } - return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, acceptLanguage, func(body *Response) (map[string]AllMailboxesResponse, Error) { - resp := map[string]AllMailboxesResponse{} + return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, acceptLanguage, func(body *Response) (map[string]Mailboxes, Error) { + resp := map[string]Mailboxes{} for _, accountId := range uniqueAccountIds { var response MailboxGetResponse err = retrieveResponseMatchParameters(logger, body, CommandMailboxGet, mcid(accountId, "0"), &response) if err != nil { - return map[string]AllMailboxesResponse{}, err + return nil, err } - resp[accountId] = AllMailboxesResponse{ + resp[accountId] = Mailboxes{ Mailboxes: response.List, State: response.State, } diff --git a/services/groupware/pkg/groupware/groupware_api_mailbox.go b/services/groupware/pkg/groupware/groupware_api_mailbox.go index 9cb5d5749..5e4953959 100644 --- a/services/groupware/pkg/groupware/groupware_api_mailbox.go +++ b/services/groupware/pkg/groupware/groupware_api_mailbox.go @@ -2,6 +2,8 @@ package groupware import ( "net/http" + "slices" + "strings" "github.com/go-chi/chi/v5" "github.com/rs/zerolog" @@ -128,9 +130,8 @@ func (g *Groupware) GetMailboxes(w http.ResponseWriter, r *http.Request) { return req.errorResponseFromJmap(err) } - mailboxes, ok := mailboxesByAccountId[accountId] - if ok { - return etagResponse(mailboxes.Mailboxes, sessionState, mailboxes.State, lang) + if mailboxes, ok := mailboxesByAccountId[accountId]; ok { + return etagResponse(sortMailboxSlice(mailboxes.Mailboxes), sessionState, mailboxes.State, lang) } else { return notFoundResponse(sessionState) } @@ -139,9 +140,8 @@ func (g *Groupware) GetMailboxes(w http.ResponseWriter, r *http.Request) { if err != nil { return req.errorResponseFromJmap(err) } - mailboxes, ok := mailboxesByAccountId[accountId] - if ok { - return etagResponse(mailboxes.Mailboxes, sessionState, mailboxes.State, lang) + if mailboxes, ok := mailboxesByAccountId[accountId]; ok { + return etagResponse(sortMailboxSlice(mailboxes.Mailboxes), sessionState, mailboxes.State, lang) } else { return notFoundResponse(sessionState) } @@ -197,13 +197,13 @@ func (g *Groupware) GetMailboxesForAllAccounts(w http.ResponseWriter, r *http.Re if err != nil { return req.errorResponseFromJmap(err) } - return response(mailboxesByAccountId, sessionState, lang) + return response(sortMailboxesMap(mailboxesByAccountId), sessionState, lang) } else { mailboxesByAccountId, sessionState, lang, err := g.jmap.GetAllMailboxes(accountIds, req.session, req.ctx, logger, req.language()) if err != nil { return req.errorResponseFromJmap(err) } - return response(mailboxesByAccountId, sessionState, lang) + return response(sortMailboxesMap(mailboxesByAccountId), sessionState, lang) } }) } @@ -225,7 +225,7 @@ func (g *Groupware) GetMailboxByRoleForAllAccounts(w http.ResponseWriter, r *htt if err != nil { return req.errorResponseFromJmap(err) } - return response(mailboxesByAccountId, sessionState, lang) + return response(sortMailboxesMap(mailboxesByAccountId), sessionState, lang) }) } @@ -344,3 +344,72 @@ func (g *Groupware) GetMailboxRoles(w http.ResponseWriter, r *http.Request) { return response(rolesByAccountId, sessionState, lang) }) } + +var mailboxRoleSortOrderScore = map[string]int{ + jmap.JmapMailboxRoleInbox: 100, + jmap.JmapMailboxRoleDrafts: 200, + jmap.JmapMailboxRoleSent: 300, + jmap.JmapMailboxRoleJunk: 400, + jmap.JmapMailboxRoleTrash: 500, +} + +func scoreMailbox(m jmap.Mailbox) int { + if score, ok := mailboxRoleSortOrderScore[m.Role]; ok { + return score + } + return 1000 +} + +func sortMailboxesMap[K comparable](mailboxesByAccountId map[K]jmap.Mailboxes) map[K]jmap.Mailboxes { + sortedByAccountId := make(map[K]jmap.Mailboxes, len(mailboxesByAccountId)) + for accountId, unsorted := range mailboxesByAccountId { + mailboxes := make([]jmap.Mailbox, len(unsorted.Mailboxes)) + copy(mailboxes, unsorted.Mailboxes) + slices.SortFunc(mailboxes, compareMailboxes) + sortedByAccountId[accountId] = jmap.Mailboxes{Mailboxes: mailboxes, State: unsorted.State} + } + return sortedByAccountId +} + +func sortMailboxSlice(s []jmap.Mailbox) []jmap.Mailbox { + r := make([]jmap.Mailbox, len(s)) + copy(r, s) + slices.SortFunc(r, compareMailboxes) + return r +} + +func compareMailboxes(a, b jmap.Mailbox) int { + // first, use the defined order: + // Defines the sort order of Mailboxes when presented in the client’s UI, so it is consistent between devices. + // Default value: 0 + // The number MUST be an integer in the range 0 <= sortOrder < 2^31. + // A Mailbox with a lower order should be displayed before a Mailbox with a higher order + // (that has the same parent) in any Mailbox listing in the client’s UI. + sa := a.SortOrder + sb := b.SortOrder + r := sa - sb + if r != 0 { + return r + } + + // the JMAP specification says this: + // > Mailboxes with equal order SHOULD be sorted in alphabetical order by name. + // > The sorting should take into account locale-specific character order convention. + // but we feel like users would rather expect standard folders to come first, + // in an order that is common across MUAs: + // - inbox + // - drafts + // - sent + // - junk + // - trash + // - *everything else* + sa = scoreMailbox(a) + sb = scoreMailbox(b) + r = sa - sb + if r != 0 { + return r + } + + // now we have "everything else", let's use alphabetical order here: + return strings.Compare(a.Name, b.Name) +} diff --git a/services/groupware/pkg/groupware/groupware_test.go b/services/groupware/pkg/groupware/groupware_test.go index ea5d70a78..a6e7fbd46 100644 --- a/services/groupware/pkg/groupware/groupware_test.go +++ b/services/groupware/pkg/groupware/groupware_test.go @@ -1,9 +1,11 @@ package groupware import ( + "slices" "testing" "github.com/opencloud-eu/opencloud/pkg/jmap" + "github.com/opencloud-eu/opencloud/pkg/structs" "github.com/stretchr/testify/require" ) @@ -44,3 +46,18 @@ func TestSanitizeEmail(t *testing.T) { require.Equal(`Hello. Click here for AI slop.`, safe.BodyValues["zee7urae"].Value) require.Equal(30, safe.HtmlBody[1].Size) } + +func TestSortMailboxes(t *testing.T) { + mailboxes := []jmap.Mailbox{ + {Id: "a", Name: "Other"}, + {Id: "b", Role: jmap.JmapMailboxRoleSent, Name: "Sent"}, + {Id: "c", Name: "Zebras"}, + {Id: "d", Role: jmap.JmapMailboxRoleInbox, Name: "Inbox"}, + {Id: "e", Name: "Appraisal"}, + {Id: "f", Name: "Zealots", SortOrder: -10}, + } + slices.SortFunc(mailboxes, compareMailboxes) + names := structs.Map(mailboxes, func(m jmap.Mailbox) string { return m.Name }) + require := require.New(t) + require.Equal([]string{"Zealots", "Inbox", "Sent", "Appraisal", "Other", "Zebras"}, names) +}