Page MenuHomePhabricator

Calendar query results should be ordered from first start date to last start date
ClosedPublic

Authored by lpriestley on May 21 2015, 5:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 22, 1:04 AM
Unknown Object (File)
Sat, Dec 21, 6:05 AM
Unknown Object (File)
Mon, Dec 16, 8:22 AM
Unknown Object (File)
Wed, Dec 11, 11:15 PM
Unknown Object (File)
Fri, Dec 6, 5:25 PM
Unknown Object (File)
Nov 29 2024, 12:47 PM
Unknown Object (File)
Nov 27 2024, 10:24 PM
Unknown Object (File)
Nov 23 2024, 11:16 PM
Subscribers

Details

Summary

Closes T8041, Calendar query results should be ordered from first start date to last start date

Test Plan

Open "Upcoming Events" in Calendar, verify that the event with the first start date is first and that events are ordered in ascending start dates.

Diff Detail

Repository
rP Phabricator
Branch
calendarqueryorder
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 6192
Build 6213: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

lpriestley retitled this revision from to Calendar query results should be ordered from first start date to last start date.
lpriestley updated this object.
lpriestley edited the test plan for this revision. (Show Details)
lpriestley added a reviewer: epriestley.
epriestley edited edge metadata.
epriestley added inline comments.
src/applications/calendar/query/PhabricatorCalendarEventQuery.php
46

(3) After you remove 'unique' => true, you'll have to change this to array('start', 'id'); or it will complain that the order vector does not have a terminal unique column.

55

(1) This should be 'int'.

56

(2) This isn't unique, since two events can have the same exact start value.

Labeling a non-unique column as unique will create problems when the edge of a page has objects with the same value on either side of it. For example, if the end of the first page has several events which start on May 3 at 12:00 AM, and the beginning of the second page also has several events which start on May 3 at 12:00 AM, the code will believe that it can exclude all of them (because it thinks the value is unique). It will generate a query like AND dateFrom > "May 3 at 12:00 AM" and skip all of the results on the second page.

58

(4) After you change the vector to array('start', 'id'), you'll get an error about 'id' not being defined as an orderable column. You can fix that by including the 'id' definition from the parent class with + parent::getOrderableColumns().

63–64

(5) Finally, after making 'id' an orderable column, this will complain that the paging value map does not include a value for 'id'. Add 'id' => $event->getID() here to fix that. Then everything should work.

This revision now requires changes to proceed.May 21 2015, 5:51 PM
lpriestley edited edge metadata.
lpriestley marked 5 inline comments as done.

Fixing implementation of orderableColumns

epriestley edited edge metadata.
This revision is now accepted and ready to land.May 21 2015, 10:54 PM
This revision was automatically updated to reflect the committed changes.