Bug 18728 - use html filter in suggestions datatable sort
Summary: use html filter in suggestions datatable sort
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-05 11:54 UTC by Fridolin Somers
Modified: 2018-06-04 20:10 UTC (History)
1 user (show)

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


Attachments
Bug 18728 - use html filter in suggestions datatable sort (2.04 KB, patch)
2017-06-05 12:01 UTC, Fridolin Somers
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18728 - use html filter in suggestions datatable sort (2.11 KB, patch)
2017-06-06 04:21 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2017-06-05 11:54:31 UTC
In suggestions datatable one can sort on colums "Suggested by" or "Managed by".
Those columns may contain HTML tags.
Use datatable sorting type html to have a correct sorting.
Comment 1 Fridolin Somers 2017-06-05 12:01:58 UTC
Created attachment 63960 [details] [review]
Bug 18728 - use html filter in suggestions datatable sort

In suggestions datatable one can sort on colums "Suggested by" or "Managed by".
Those columns may contain HTML tags.
Use datatable sorting type html to have a correct sorting.

Test plan :
- Be sure to have some suggestions asked and managed
- Go to Acquisitions > Suggestions
- Click on "Suggested by" to sort
=> Table is sorted by the user name
- Click on "Managed by" to sort
=> Table is sorted by the user name
Comment 2 Josef Moravec 2017-06-06 04:21:38 UTC
Created attachment 64018 [details] [review]
[SIGNED-OFF] Bug 18728 - use html filter in suggestions datatable sort

In suggestions datatable one can sort on colums "Suggested by" or "Managed by".
Those columns may contain HTML tags.
Use datatable sorting type html to have a correct sorting.

Test plan :
- Be sure to have some suggestions asked and managed
- Go to Acquisitions > Suggestions
- Click on "Suggested by" to sort
=> Table is sorted by the user name
- Click on "Managed by" to sort
=> Table is sorted by the user name

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 3 Jonathan Druart 2017-06-09 13:57:23 UTC
> Those columns may contain HTML tags.

How?
Comment 4 Owen Leonard 2017-06-09 15:21:47 UTC
The user name in each column is a link:

<td>
   <a href="mailto:oleonard@example.com">Smith, Dave (0123456)</a> - 09/08/2016
</td>

...so there is definitely HTML, but I don't find that it sorts incorrectly. Are there only certain circumstances which trigger a bug?
Comment 5 Fridolin Somers 2017-06-13 10:44:45 UTC
(In reply to Owen Leonard from comment #4)
> The user name in each column is a link:
> 
> <td>
>    <a href="mailto:oleonard@example.com">Smith, Dave (0123456)</a> -
> 09/08/2016
> </td>
> 
> ...so there is definitely HTML, but I don't find that it sorts incorrectly.
> Are there only certain circumstances which trigger a bug?

Try with some users without email and some with email :
Doe, John
Smith, John
<a href="mailto:jannesmith@example.com">Smith, Jane</a>
Comment 6 Jonathan Druart 2017-06-13 15:31:06 UTC
Sounds like it is already correctly sorted without the patch.

Asking for another signoff.
Comment 7 Jonathan Druart 2017-06-13 15:32:14 UTC
I used the following script to generate the data:

use Modern::Perl;
use t::lib::TestBuilder;
my $builder = t::lib::TestBuilder->new;

my $type = 'Suggestion';
my $number = 100;

if ( $type eq 'Suggestion' ) {
    for ( 1 .. $number ) {
        my $managed_by = $builder->build({ source => 'Borrower' });
        my $suggested_by = $builder->build({ source => 'Borrower' });
        $builder->build({ source => 'Suggestion', value => { STATUS => 'pending', managedby => $managed_by->{borrowernumber}, suggestedby => $suggested_by->{borrowernumber} } });
    }
}
Comment 8 Fridolin Somers 2017-06-14 08:42:17 UTC
(In reply to Jonathan Druart from comment #6)
> Sounds like it is already correctly sorted without the patch.
> 
> Asking for another signoff.

Indeed, I can't reproduce.
Maybe something changed in DataTable JS.
Comment 9 Fridolin Somers 2017-06-14 08:51:18 UTC
Ah when it does not work, sort is on member email (that is inside href) instead of name.
Even with all cells with html <a></a>
Comment 10 Fridolin Somers 2017-06-14 08:56:24 UTC
(In reply to Fridolin SOMERS from comment #9)
> Ah when it does not work, sort is on member email (that is inside href)
> instead of name.
> Even with all cells with html <a></a>

I can reproduce on 3.22.

I can reproduce now on master : 

Suggested by :
<a href="mailto:zzz@xxx.fr">DIFRON, Daniel (90519000019877)</a>
<a href="mailto:aaa@xxx.fr">NAPISTE, Nicole (90519000014977)</a>

It is well sorted : DIFRON than NAPISTE.

If anyone can confirm I'll set has resolved.
Comment 11 Owen Leonard 2017-06-14 14:38:44 UTC
I still can't reproduce, even after remove some email addresses from patron records, adding different email addresses to identically-named patrons, etc. I think this is resolved.
Comment 12 Fridolin Somers 2017-06-14 14:42:25 UTC
Oki, thanks all for testing.
I mark as resolved.
Comment 13 Jonathan Druart 2017-06-14 15:06:53 UTC
Next time please confirm the bug before fixing it ;)