Contacts API: Add Sorting

RESOLVED FIXED in mozilla14

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gwagner, Assigned: gwagner)

Tracking

unspecified
mozilla14
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

14.89 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
We should support sorting by field for find results. Like { sortBy: familyName, sortOrder: descending}.
(Assignee)

Updated

5 years ago
Assignee: nobody → anygregor
(Assignee)

Comment 1

5 years ago
Created attachment 611097 [details] [diff] [review]
patch
(Assignee)

Comment 2

5 years ago
Created attachment 611559 [details] [diff] [review]
patch
Attachment #611097 - Attachment is obsolete: true
Attachment #611559 - Flags: review?(bent.mozilla)
Comment on attachment 611559 [details] [diff] [review]
patch

Review of attachment 611559 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/contacts/fallback/ContactDB.jsm
@@ +377,5 @@
>        }
>      }
>  
> +    // Sorting functions takes care of limit if set.
> +    let limit = options.sortBy !== 'undefined' ? null : options.filterLimit;

Hm, should that be testing against 'undefined'? Looks like you always define it to '' if it wasn't passed in...

Also, any time you are going to do something based on both branches of an if statement (in this case the ? operator) I think it's nicer to test against a non-negated condition (e.g. |options.sortBy === 'undefined' ? options.filterLimit : null|).

@@ +413,5 @@
>      debug("ContactDB:_findAll:  " + JSON.stringify(options));
>      if (!txn.result)
>        txn.result = {};
> +    // Sorting functions takes care of limit if set.
> +    let limit = options.sortBy !== 'undefined' ? null : options.filterLimit;

here too.
(Assignee)

Comment 4

5 years ago
(In reply to ben turner [:bent] from comment #3)
> Comment on attachment 611559 [details] [diff] [review]
> patch
> 
> Review of attachment 611559 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/contacts/fallback/ContactDB.jsm
> @@ +377,5 @@
> >        }
> >      }
> >  
> > +    // Sorting functions takes care of limit if set.
> > +    let limit = options.sortBy !== 'undefined' ? null : options.filterLimit;
> 
> Hm, should that be testing against 'undefined'? Looks like you always define
> it to '' if it wasn't passed in...
> 
> Also, any time you are going to do something based on both branches of an if
> statement (in this case the ? operator) I think it's nicer to test against a
> non-negated condition (e.g. |options.sortBy === 'undefined' ?
> options.filterLimit : null|).

Actually the options object isn't created by the constructor. So the values are undefined.
(Assignee)

Comment 5

5 years ago
Created attachment 611622 [details] [diff] [review]
patch
Attachment #611559 - Attachment is obsolete: true
Attachment #611622 - Flags: review?(bent.mozilla)
Attachment #611559 - Flags: review?(bent.mozilla)
Comment on attachment 611622 [details] [diff] [review]
patch

Review of attachment 611622 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/contacts/ContactManager.js
@@ +18,5 @@
>  Cu.import("resource://gre/modules/XPCOMUtils.jsm");
>  Cu.import("resource://gre/modules/Services.jsm");
>  Cu.import("resource://gre/modules/DOMRequestHelper.jsm");
>  
> +XPCOMUtils.defineLazyGetter(Services, "DOMRequest", function() {

This change doesn't seem strictly related, but ok.
Attachment #611622 - Flags: review?(bent.mozilla) → review+
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ad6cc527d5c
https://hg.mozilla.org/mozilla-central/rev/3ad6cc527d5c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.