Skip to content

added displayfrom and displayto search criteria#721

Open
FlexibleDevs wants to merge 6 commits into
mscdex:masterfrom
FlexibleDevs:master
Open

added displayfrom and displayto search criteria#721
FlexibleDevs wants to merge 6 commits into
mscdex:masterfrom
FlexibleDevs:master

Conversation

@FlexibleDevs

Copy link
Copy Markdown

Hi, I'm Filippo and i'm using your amazing lib for connecting my project to an imap server.
I've modified the list of sort criteria to implement the DISPLAYFROM and TO (as described here: https://tools.ietf.org/html/rfc5957). I think that the 2 added lines should be enough to implement that.

Could you review my branch and eventually merge it into master ?

Thanks for you work, I'm happy to contribute.

Let me know,
Filippo

@mscdex

mscdex commented Sep 3, 2018

Copy link
Copy Markdown
Owner

We need to be more strict and ensure that the server supports the 'SORT=DISPLAY' capability before allowing these sort options.

@FlexibleDevs

Copy link
Copy Markdown
Author

I see, it's correct.
Can i use your serverSupports function? Maybe something like that "if (!this.serverSupports('THREAD=' + algorithm))"
I'll modify the code to check "sort=display" capability.

Let me know,
thanks for your fast response 👍

Filippo

@FlexibleDevs

Copy link
Copy Markdown
Author

Modified the sort switch to control server capability.
Could you check the modified version ?

Filippo

@matteomattei

Copy link
Copy Markdown

+1

@mscdex

mscdex commented Sep 11, 2018

Copy link
Copy Markdown
Owner

It looks like there are other non-related changes now?

@matteomattei

matteomattei commented Sep 11, 2018

Copy link
Copy Markdown

This should be the final patch

index 619862f..c9119f3 100644
--- a/lib/Connection.js
+++ b/lib/Connection.js
@@ -917,6 +917,7 @@ Connection.prototype.sort = function(sorts, criteria, cb) {
 };
 
 Connection.prototype._sort = function(which, sorts, criteria, cb) {
+  let displaySupported = false;
   if (this._box === undefined)
     throw new Error('No mailbox is currently selected');
   else if (!Array.isArray(sorts) || !sorts.length)
@@ -925,6 +926,9 @@ Connection.prototype._sort = function(which, sorts, criteria, cb) {
     throw new Error('Expected array for search criteria');
   else if (!this.serverSupports('SORT'))
     throw new Error('Sort is not supported on the server');
+  if(this.serverSupports("SORT=DISPLAY")){
+    displaySupported = true;
+  }
 
   sorts = sorts.map(function(c) {
     if (typeof c !== 'string')
@@ -945,6 +949,12 @@ Connection.prototype._sort = function(which, sorts, criteria, cb) {
       case 'SUBJECT':
       case 'TO':
         break;
+      case 'DISPLAYFROM':
+      case 'DISPLAYTO':
+        if(!displaySupported){
+          throw new Error("Unexpected sort criteria: " + c);
+        }
+        break;
       default:
         throw new Error('Unexpected sort criteria: ' + c);
     }

@mauriziomambrini

Copy link
Copy Markdown

+1

1 similar comment
@Carlobravo

Copy link
Copy Markdown

+1

@alessice

Copy link
Copy Markdown

+1 I'm using Dovecot as IMAP server and it support this IMAP extension. Since Dovecot have a 75% market share is a very useful patch for many people.

@ciaoben

ciaoben commented Sep 17, 2018

Copy link
Copy Markdown

+1

1 similar comment
@JonahKr

JonahKr commented Oct 13, 2018

Copy link
Copy Markdown

+1

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

Successfully merging this pull request may close these issues.

9 participants