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

[SearchSnippet] SearchSnippet/get method #5254

Closed
Arsnael opened this issue Aug 27, 2024 · 11 comments · Fixed by apache/james-project#2442
Closed

[SearchSnippet] SearchSnippet/get method #5254

Arsnael opened this issue Aug 27, 2024 · 11 comments · Fixed by apache/james-project#2442
Assignees
Labels
Milestone

Comments

@Arsnael
Copy link
Member

Arsnael commented Aug 27, 2024

Implement a new JMAP method as defined in https://jmap.io/spec-mail.html#search-snippets

Add a SearchSnippet object as defined per the RFC and implement the SearchSnippetGetMethod, that is similar to EmailQueryMethod, except that it would call the search with the highlight instead of normal search function.

Notes: Change in SearchSnippet internal jmap doc Not implemented to Implemented

DoD: Integration tests

@vttranlina
Copy link
Member

I understand this ticket is implemented according to the jmap spec, but I have a related concern.

The SearchSnippet object has only property:

  • emailId
  • subject
  • preview

I think it is not enough for FE,
Now, the request body when we "Search email" (tmail-web):

single jmap request:

  • "Email/query" : for get id
  • "Email/get" (backReference query): for get property: "id", "blobId", "subject", "from", "to", "cc", "bcc",
    "keywords", "size", "receivedAt", "sentAt", "preview",
    "hasAttachment", "replyTo", "mailboxIds",
    "header:X-MEETING-UID:asText"

If the frontend uses SearchSnippet/get, the subject and preview will be retrieved from SearchSnippet/get, while the other property will be obtained from Email/get.

Is it no problem?

CC: @hoangdat

@Arsnael
Copy link
Member Author

Arsnael commented Aug 28, 2024

I get the concern but I don't think we should go out bonds of the RFC spec either.

Not all properties that the front is using for email/get during a search is being shown on the screen btw, that could probably be optimized

@quantranhong1999
Copy link
Member

Notes: Change in SearchSnippet internal jmap doc Not implemented to Implemented

@hoangdat
Copy link
Member

Hi, from my understanding, FE can combine

Email/query
Email/get
SearchSnippet/get

And combine the response of get and searchsnippet to show to FE. No problem.

@chibenwa
Copy link
Member

The preview in SearchSnipet is not the preview of the message (as in email/get), but a subset of the relevant part of the body.

Overall agrees.

@dab246
Copy link
Member

dab246 commented Aug 29, 2024

Hi, from my understanding, FE can combine

Email/query
Email/get
SearchSnippet/get

And combine the response of get and searchsnippet to show to FE. No problem.

IMO, It will take us more time to combine it. Is there a better solution? I see SearchSnippet/get is taking a list of email ids emailIds. So does its response return the list of results in the order of the email ids passed in?

@Arsnael
Copy link
Member Author

Arsnael commented Aug 29, 2024

IMO, It will take us more time to combine it. Is there a better solution? I see SearchSnippet/get is taking a list of email ids emailIds. So does its response return the list of results in the order of the email ids passed in?

The RFC says that it might not be responded in the same order as given in the request, I am unsure how complex it would be but I guess we could try?

@chibenwa
Copy link
Member

The RFC says that it might not be responded in the same order as given in the request, I am unsure how complex it would be but I guess we could try?

Like between email/query + email/get. Front should ensure the odering displayed is the one of email/query

@vttranlina
Copy link
Member

WIP apache#2442

@vttranlina
Copy link
Member

I forgot this ticket isn't a priority in this sprint, so I'll pause it for now.

@vttranlina
Copy link
Member

Split tasks

And this ticket will focus on implement SearchSnippet/get for distributed

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

Successfully merging a pull request may close this issue.

6 participants