#2708 Sorting on fields in Issues table
Merged by pingou. Opened by mohanboddu.
mohanboddu/pagure mohan-pagure-fixes  into  master

Download 2708.patch

This PR is separated out in four commits:

  • The first and second commit are adding the sort logic
  • The third commit removes "stupidtable" since it is mostly not used anymore and didn't support pagination
  • The fourth commit adds an arrow showing the direction that you are sorting the column by. The arrow is from font-awesome, which was added here.

rebased onto c4d89cc2cf2608cc373efad7bf27c8db237bf9da

Would be nice to add some tests for this :)

Any chances we could rely on openiconic rather than re-adding font-awesome (that we removed a while back)?

Any chances we could rely on openiconic rather than re-adding font-awesome (that we removed a while back)?

Sure, I didn't know about that.

@mohanboddu, can you replace the last commit with this commit instead?
https://paste.fedoraproject.org/paste/3ywf5xWQjHITLOaN1750Tg

4 new commits added

  • Add arrow directions in sorting
  • Remove "stupidtable" since almost all of the functionality was replaced by database sorting and it doesn't support pagination
  • Clean up the issue sorting links
  • Sorting on Opened, Modified, Closed, Priority, Reporter, Assignee cols

@pingou your comment about font-awesome was addressed.

@mohanboddu can you squash this patch into commit c6c422c4d1d489b81382cd1de31e75d1dfe5065b?:
https://paste.fedoraproject.org/paste/p12XUfe1t6ZfmoT51rYqjA

Then can you add this commit which includes the unit tests?
https://paste.fedoraproject.org/paste/IHLCDqjhVkV-B-BPJabHqQ

4 new commits added

  • Add unit tests for sorting issues
  • Remove "stupidtable" since almost all of the functionality was replaced by database sorting and it doesn't support pagination
  • Clean up the issue sorting links
  • Sorting on Opened, Modified, Closed, Priority, Reporter, Assignee cols

@pingou Added unit tests.

Thanks @mprahl.

Could we change this to jinja filters? See pagure/ui/filters.py

4 new commits added

  • Add unit tests for sorting issues
  • Remove "stupidtable" since almost all of the functionality was replaced by database sorting and it doesn't support pagination
  • Clean up the issue sorting links
  • Sorting on Opened, Modified, Closed, Priority, Reporter, Assignee cols

To get it working with postgresql I have had to make the following changes:

diff --git a/ pagure/lib/__init__.py b/ pagure/lib/__init__.py
index 80374fc..2eb02ef 100644
--- a/ pagure/lib/__init__.py   
+++ b/ pagure/lib/__init__.py   
@@ -2351,13 +2351,16 @@ def search_issues(
     if order_key and order_key in model.Issue.__table__.columns.keys():
         column = getattr(model.Issue, order_key)
+    if str(column.type) == 'TEXT':
+        column = func.lower(column)
+
     # The priority is sorted differently because it is by weight and the lower
     # the number, the higher the priority
     if (order_key != 'priority' and order == 'asc') or \
             (order_key == 'priority' and order == 'desc'):
-        query = query.order_by(asc(func.lower(column)))
+        query = query.order_by(asc(column))
     else:
-        query = query.order_by(desc(func.lower(column)))
+        query = query.order_by(desc(column))
     if issueid is not None or issueuid is not None:
         output = query.first()
diff --git a/ pagure/templates/issues.html b/ pagure/templates/issues.html
index 7096fdc..286999c 100644
--- a/ pagure/templates/issues.html 
+++ b/ pagure/templates/issues.html 
@@ -162,7 +162,7 @@
             {{ 'title' | table_sort_arrow(order_key, order) | safe }}</th>
           <th class="open_date"><a href="{{ url_for('view_issues', **dict(
             base_url_for, status=status or 'all', order_key='date_created',
-            order='date_created' | table_get_link_order(order_key, order))) }}">Opened</a> 
+            order='date_created' | table_get_link_order(order_key, order))) }}">Opened</a>
             {{ 'date_created' | table_sort_arrow(order_key, order) | safe }}</th>
           {% if status and status|lower != 'open' %}
           <th class="close_date"><a href="{{ url_for('view_issues', **dict(
@@ -180,14 +180,14 @@
             order='priority' | table_get_link_order(order_key, order))) }}">Priority</a>
               {{ 'priority' | table_sort_arrow(order_key, order) | safe }}</th>
           <th class="open_by"><a href="{{ url_for('view_issues', **dict(
-            base_url_for, status=status or 'all', order_key='author',
-            order='author' | table_get_link_order(order_key, order))) }}">Reporter</a>
-            {{ 'author' | table_sort_arrow(order_key, order) | safe }}</th>
+            base_url_for, status=status or 'all', order_key='author_id',
+            order='author_id' | table_get_link_order(order_key, order))) }}">Reporter</a>
+            {{ 'author_id' | table_sort_arrow(order_key, order) | safe }}</th>
           {% if not status or status|lower == 'open' %}
           <th class="assigned"><a href="{{ url_for('view_issues', **dict(
-            base_url_for, status=status or 'all', order_key='assignee',
-            order='assignee' | table_get_link_order(order_key, order))) }}">Assignee</a>
-            {{ 'assignee' | table_sort_arrow(order_key, order) | safe }}</th>
+            base_url_for, status=status or 'all', order_key='assignee_id',
+            order='assignee_id' | table_get_link_order(order_key, order))) }}">Assignee</a>
+            {{ 'assignee_id' | table_sort_arrow(order_key, order) | safe }}</th>
           {% endif %}
         </tr>
     </thead>

@pingou, thank you for the patch. That helped except for ordering by assignee_id and author_id. I rebased the PR to sort by the assignee's username and user's username (author). I added unit tests to cover this. I hope this works on PostgreSQL as it worked on SQLite.

4 new commits added

  • Add unit tests for sorting issues
  • Remove "stupidtable" since almost all of the functionality was replaced by database sorting and it doesn't support pagination
  • Clean up the issue sorting links
  • Sorting on Opened, Modified, Closed, Priority, Reporter, Assignee cols

Seems to work as desired, let's rebase and merge :)

rebased onto c073b40fe1d59282e73cdf0c70ade6266a46cccc

Pull-Request has been merged by pingou

Metadata