Get requests is sorted according to last_modified

[ayusharma]

This solves the issue Kinto/kinto#434 and sorts the records according to the last_modified attribute.

[Natim]

We should definitely add a test that should have been failling without your patch so that if someones removes this line in the future we will have a test for that.

[ayusharma]

Is my solution to this bug correct ? Should I write test case for this ? Guide me for that.

[leplatrem]

Thanks @ayusharma for taking the time to investigate this!

I think we can just modify the extract_sorting method here to sort by last modified desc by default (and not only when limit is set):
https://github.com/mozilla-services/cliquet/blob/master/cliquet/resource/__init__.py#L943

Should I write test case for this ?

Yes, tests are the specifications of the application behaviour, therefore we should add a test that makes the default sorting behaviour explicit.

Add a method here like test_default_sort_is_last_modified_desc_by_default
https://github.com/mozilla-services/cliquet/blob/master/cliquet/tests/resource/test_sort.py

You can also add another test to assert the behaviour when a sort order field is given (e.g. -title) and several records have the same value for that field. (i.e. do we want to order equal records by last-modified too?)

Good luck and enjoy :) Let us know if you have any question !

[ayusharma]

Thanks a lot :)

In short I have to remove the limit parameter and make this function work on every condition.

if not modified_field_used and limit:
            # Add a sort by the ``modified_field`` in descending order
            # useful for pagination
            sorting.append(Sort(self.model.modified_field, -1))
        return sorting

and this would get transform to

if not modified_field_used:
            # Add a sort by the ``modified_field`` in descending order
            # useful for pagination
            sorting.append(Sort(self.model.modified_field, -1))
        return sorting

Is it right ?

[leplatrem]

Yep !

Then update tests and changelog :)

[ayusharma]

How can I write the another test to assert the behaviour when a sort order field is given (e.g. -title) and several records have the same value for that field ? means we have different testdata in records with different titles. (We need same titles to perform test operation)

and what does this function do self.patch_known_field.stop() ?

[almet]

You can just add the items to the storage yourself using the create_record method.

The way I would do it is by adding a new method to the class that inserts a specific record given its it

def _add_record(self, i):
    record = {
        'title': 'MoFo #{0:02}'.format(i),
        'status': i % 4,
        'unread': (i % 2 == 0)
    }
    self.model.create_crecord(record)

and then call it where you need, with an title that already exists (1 for instance).

[leplatrem]

How can I write the another test to assert the behaviour when a sort order field is given (e.g. -title)

If you look at the SortingTest you will see that it creates several records with the same status.

You can use a sort on status to assert that records are sorted by status and when two records have the same status, they will be sorted by last modified.

[leplatrem]

Great! I merged it :) I just made a small adjustment about an assertion in the tests