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

added displayfrom and displayto search criteria #721

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

FlexibleDevs
Copy link

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

mscdex commented Sep 3, 2018

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

@FlexibleDevs
Copy link
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
Author

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

Filippo

@matteomattei
Copy link

+1

@mscdex
Copy link
Owner

mscdex commented Sep 11, 2018

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

@matteomattei
Copy link

matteomattei commented Sep 11, 2018

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

+1

1 similar comment
@Carlobravo
Copy link

+1

@alessice
Copy link

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

ciaoben commented Sep 17, 2018

+1

1 similar comment
@JonahKr
Copy link

JonahKr commented Oct 13, 2018

+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