Bug 19277 - TT syntax - Data is not ordered in notices
Summary: TT syntax - Data is not ordered in notices
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 17966
Blocks:
  Show dependency treegraph
 
Reported: 2017-09-08 12:37 UTC by Jonathan Druart
Modified: 2019-06-27 09:24 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 19277: TT syntax - Display ordered data (2.21 KB, patch)
2017-09-08 12:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19277: Make sure the tests will pass even if they are slow (3.92 KB, patch)
2017-09-08 12:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19277: TT syntax - Display ordered data (2.25 KB, patch)
2017-09-08 16:48 UTC, Lee Jamison
Details | Diff | Splinter Review
Bug 19277: Make sure the tests will pass even if they are slow (3.96 KB, patch)
2017-09-08 16:49 UTC, Lee Jamison
Details | Diff | Splinter Review
Bug 19277: TT syntax - Display ordered data (2.31 KB, patch)
2017-09-11 14:21 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19277: Make sure the tests will pass even if they are slow (4.01 KB, patch)
2017-09-11 14:21 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2017-09-08 12:37:27 UTC
This issue has been highlighted by a failing test: ISSUESLIP displays checkouts in random order

I thought it was because of dates comparison, but it comes from the following line in C4::Letters::_get_tt_params
  my $objects = $module->search( { $key => { -in => $values } } );

The DBMS will return data like there is ordered in the DB.
For instance:
  select borrowernumber from borrowers where borrowernumber in (5, 3, 1);
or
  select borrowernumber from borrowers where borrowernumber=5 or borrowernumber=3 or borrowernumber=1;

will return 1, 3, 5
Comment 1 Jonathan Druart 2017-09-08 12:46:12 UTC
Created attachment 66985 [details] [review]
Bug 19277: TT syntax - Display ordered data

This issue has been highlighted by a failing test: ISSUESLIP displays checkouts in random order

I thought it was because of dates comparison, but it comes from the following line in C4::Letters::_get_tt_params
  my $objects = $module->search( { $key => { -in => $values } } );

The DBMS will return data like there is ordered in the DB.
For instance:
  select borrowernumber from borrowers where borrowernumber in (5, 3, 1);
or
  select borrowernumber from borrowers where borrowernumber=5 or borrowernumber=3 or borrowernumber=1;

will return 1, 3, 5

I did not find a generic way to do that, so used "ORDER BY FIELD" which will not be portable.

Test plan:
If you do not apply this patch, the tests will sometime fail
Comment 2 Jonathan Druart 2017-09-08 12:46:17 UTC
Created attachment 66986 [details] [review]
Bug 19277: Make sure the tests will pass even if they are slow

This patch was my first attempt to fix the issue.
I think it is good to have it, if issue.timestamp and issue.issuedate are the same,
the result will be orderd by issue_id.
The tests highlight the fact that checkouts must be displayed in the
correct order.
Comment 3 Lee Jamison 2017-09-08 16:48:50 UTC
Created attachment 67000 [details] [review]
Bug 19277: TT syntax - Display ordered data

This issue has been highlighted by a failing test: ISSUESLIP displays checkouts in random order

I thought it was because of dates comparison, but it comes from the following line in C4::Letters::_get_tt_params
  my $objects = $module->search( { $key => { -in => $values } } );

The DBMS will return data like there is ordered in the DB.
For instance:
  select borrowernumber from borrowers where borrowernumber in (5, 3, 1);
or
  select borrowernumber from borrowers where borrowernumber=5 or borrowernumber=3 or borrowernumber=1;

will return 1, 3, 5

I did not find a generic way to do that, so used "ORDER BY FIELD" which will not be portable.

Test plan:
If you do not apply this patch, the tests will sometime fail

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Comment 4 Lee Jamison 2017-09-08 16:49:01 UTC
Created attachment 67001 [details] [review]
Bug 19277: Make sure the tests will pass even if they are slow

This patch was my first attempt to fix the issue.
I think it is good to have it, if issue.timestamp and issue.issuedate are the same,
the result will be orderd by issue_id.
The tests highlight the fact that checkouts must be displayed in the
correct order.

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Comment 5 Tomás Cohen Arazi 2017-09-11 14:21:00 UTC
Created attachment 67043 [details] [review]
Bug 19277: TT syntax - Display ordered data

This issue has been highlighted by a failing test: ISSUESLIP displays checkouts in random order

I thought it was because of dates comparison, but it comes from the following line in C4::Letters::_get_tt_params
  my $objects = $module->search( { $key => { -in => $values } } );

The DBMS will return data like there is ordered in the DB.
For instance:
  select borrowernumber from borrowers where borrowernumber in (5, 3, 1);
or
  select borrowernumber from borrowers where borrowernumber=5 or borrowernumber=3 or borrowernumber=1;

will return 1, 3, 5

I did not find a generic way to do that, so used "ORDER BY FIELD" which will not be portable.

Test plan:
If you do not apply this patch, the tests will sometime fail

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi 2017-09-11 14:21:05 UTC
Created attachment 67044 [details] [review]
Bug 19277: Make sure the tests will pass even if they are slow

This patch was my first attempt to fix the issue.
I think it is good to have it, if issue.timestamp and issue.issuedate are the same,
the result will be orderd by issue_id.
The tests highlight the fact that checkouts must be displayed in the
correct order.

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Jonathan Druart 2017-09-12 15:13:39 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 8 Fridolin Somers 2017-09-19 15:08:25 UTC
Depends on Bug 17966 not in 17.05.x