~sircmpwn/sr.ht-dev

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
3 3

[PATCH todo.sr.ht 0/1] Implement ticket ordering

Details
Message ID
<20201006055422.209720-1-ivan@habunek.com>
DKIM signature
pass
Download raw message
This adds ordering by id and updated, using ! for reversing, similar to
negation of terms. E.g. sort:updated, !sort:updated.

I wanted to add ordering by number of comments, but it required changing
the base query to include the comment count, which was a pain to get to
work correctly using sqlalchemy so leaving it for later.

Ivan Habunek (1):
  Implement ticket ordering

 tests/test_search.py           | 115 ++++++++++++++++++++++-----------
 todosrht/blueprints/tracker.py |   3 +-
 todosrht/search.py             |  32 ++++++++-
 3 files changed, 109 insertions(+), 41 deletions(-)

-- 
2.28.0

[PATCH todo.sr.ht 1/1] Implement ticket ordering

Details
Message ID
<20201006055422.209720-2-ivan@habunek.com>
In-Reply-To
<20201006055422.209720-1-ivan@habunek.com> (view parent)
DKIM signature
pass
Download raw message
Patch: +109 -41
By default order tickets by last updated, if not otherwise defined.
---
 tests/test_search.py           | 115 ++++++++++++++++++++++-----------
 todosrht/blueprints/tracker.py |   3 +-
 todosrht/search.py             |  32 ++++++++-
 3 files changed, 109 insertions(+), 41 deletions(-)

diff --git a/tests/test_search.py b/tests/test_search.py
index 5f87887..624abf5 100644
--- a/tests/test_search.py
+++ b/tests/test_search.py
@@ -1,5 +1,6 @@
import pytest

from datetime import datetime
from tests import factories as f
from todosrht.search import apply_search
from todosrht.types import Ticket, TicketStatus
@@ -95,8 +96,8 @@ def test_ticket_search(client):
    assert search("description_4") == [ticket4]
    assert search("description_5") == [ticket5]

    assert search("lightsabre") == [ticket1, ticket3, ticket5]
    assert search("blaster") == [ticket2, ticket4]
    assert search("lightsabre") == [ticket5, ticket3, ticket1]
    assert search("blaster") == [ticket4, ticket2]

    # Search by comment
    assert search("parsecs") == [ticket1]
@@ -110,29 +111,29 @@ def test_ticket_search(client):
    assert search("'force may with you be'") == []

    # Search either title, description, or comment
    assert search("used_to_test_or") == [ticket1, ticket2, ticket3]
    assert search("used_to_test_or") == [ticket3, ticket2, ticket1]

    # Search by submitter
    assert search("submitter:luke") == [ticket3]
    assert search("submitter:leia") == [ticket4, ticket5]
    assert search("submitter:han") == [ticket1, ticket2]
    assert search("submitter:leia") == [ticket5, ticket4]
    assert search("submitter:han") == [ticket2, ticket1]

    assert search("!submitter:luke") == [ticket1, ticket2, ticket4, ticket5]
    assert search("!submitter:leia") == [ticket1, ticket2, ticket3]
    assert search("!submitter:han") == [ticket3, ticket4, ticket5]
    assert search("!submitter:luke") == [ticket5, ticket4, ticket2, ticket1]
    assert search("!submitter:leia") == [ticket3, ticket2, ticket1]
    assert search("!submitter:han") == [ticket5, ticket4, ticket3]

    # Search by asignee
    assert search("assigned:luke") == [ticket1, ticket2]
    assert search("assigned:leia") == [ticket2, ticket3]
    assert search("!assigned:luke") == [ticket3, ticket4, ticket5]
    assert search("!assigned:leia") == [ticket1, ticket4, ticket5]
    assert search("assigned:luke") == [ticket2, ticket1]
    assert search("assigned:leia") == [ticket3, ticket2]
    assert search("!assigned:luke") == [ticket5, ticket4, ticket3]
    assert search("!assigned:leia") == [ticket5, ticket4, ticket1]
    assert search("assigned:luke assigned:leia") == [ticket2]
    assert search("assigned:luke !assigned:leia") == [ticket1]
    assert search("!assigned:luke assigned:leia") == [ticket3]
    assert search("!assigned:luke !assigned:leia") == [ticket4, ticket5]
    assert search("!assigned:luke !assigned:leia") == [ticket5, ticket4]

    assert search("no:assignee") == [ticket4, ticket5]
    assert search("!no:assignee") == [ticket1, ticket2, ticket3]
    assert search("no:assignee") == [ticket5, ticket4]
    assert search("!no:assignee") == [ticket3, ticket2, ticket1]

    with pytest.raises(ValueError) as excinfo:
        search("no:foo")
@@ -140,23 +141,23 @@ def test_ticket_search(client):

    assert search("assigned:me") == []
    assert search("assigned:me", han.user) == []
    assert search("assigned:me", luke.user) == [ticket1, ticket2]
    assert search("assigned:me", leia.user) == [ticket2, ticket3]
    assert search("assigned:me", luke.user) == [ticket2, ticket1]
    assert search("assigned:me", leia.user) == [ticket3, ticket2]

    assert search("!assigned:me") == [ticket1, ticket2, ticket3, ticket4, ticket5]
    assert search("!assigned:me", han.user) == [ticket1, ticket2, ticket3, ticket4, ticket5]
    assert search("!assigned:me", luke.user) == [ticket3, ticket4, ticket5]
    assert search("!assigned:me", leia.user) == [ticket1, ticket4, ticket5]
    assert search("!assigned:me") == [ticket5, ticket4, ticket3, ticket2, ticket1]
    assert search("!assigned:me", han.user) == [ticket5, ticket4, ticket3, ticket2, ticket1]
    assert search("!assigned:me", luke.user) == [ticket5, ticket4, ticket3]
    assert search("!assigned:me", leia.user) == [ticket5, ticket4, ticket1]

    # Search by label
    assert search("label:jedi") == [ticket1, ticket5]
    assert search("label:sith") == [ticket2, ticket5]
    assert search("!label:jedi") == [ticket2, ticket3, ticket4]
    assert search("!label:sith") == [ticket1, ticket3, ticket4]
    assert search("label:jedi") == [ticket5, ticket1]
    assert search("label:sith") == [ticket5, ticket2]
    assert search("!label:jedi") == [ticket4, ticket3, ticket2]
    assert search("!label:sith") == [ticket4, ticket3, ticket1]
    assert search("label:jedi label:sith") == [ticket5]

    assert search("no:label") == [ticket3, ticket4]
    assert search("!no:label") == [ticket1, ticket2, ticket5]
    assert search("no:label") == [ticket4, ticket3]
    assert search("!no:label") == [ticket5, ticket2, ticket1]

    # Combinations
    assert search(
@@ -184,14 +185,14 @@ def test_ticket_search_by_status(client):
    def search(search_string, user=owner):
        return apply_search(query, search_string, user).all()

    assert search("") == [ticket1, ticket2, ticket3, ticket4]
    assert search("status:any") == [ticket1, ticket2, ticket3, ticket4, ticket5]
    assert search("status:open") == [ticket1, ticket2, ticket3, ticket4]
    assert search("") == [ticket4, ticket3, ticket2, ticket1]
    assert search("status:any") == [ticket5, ticket4, ticket3, ticket2, ticket1]
    assert search("status:open") == [ticket4, ticket3, ticket2, ticket1]
    assert search("status:closed") == [ticket5]

    assert search("!status:any") == []
    assert search("!status:open") == [ticket5]
    assert search("!status:closed") == [ticket1, ticket2, ticket3, ticket4]
    assert search("!status:closed") == [ticket4, ticket3, ticket2, ticket1]

    assert search("status:reported") == [ticket1]
    assert search("status:confirmed") == [ticket2]
@@ -199,12 +200,54 @@ def test_ticket_search_by_status(client):
    assert search("status:pending") == [ticket4]
    assert search("status:resolved") == [ticket5]

    assert search("!status:reported") == [ticket2, ticket3, ticket4, ticket5]
    assert search("!status:confirmed") == [ticket1, ticket3, ticket4, ticket5]
    assert search("!status:in_progress") == [ticket1, ticket2, ticket4, ticket5]
    assert search("!status:pending") == [ticket1, ticket2, ticket3, ticket5]
    assert search("!status:resolved") == [ticket1, ticket2, ticket3, ticket4]
    assert search("!status:reported") == [ticket5, ticket4, ticket3, ticket2]
    assert search("!status:confirmed") == [ticket5, ticket4, ticket3, ticket1]
    assert search("!status:in_progress") == [ticket5, ticket4, ticket2, ticket1]
    assert search("!status:pending") == [ticket5, ticket3, ticket2, ticket1]
    assert search("!status:resolved") == [ticket4, ticket3, ticket2, ticket1]

    with pytest.raises(ValueError) as excinfo:
        search("status:foo")
    assert str(excinfo.value) == "Invalid status: 'foo'"

def test_sorting(client):
    owner = f.UserFactory()
    tracker = f.TrackerFactory(owner=owner)

    ticket1 = f.TicketFactory(tracker=tracker)
    ticket2 = f.TicketFactory(tracker=tracker)
    ticket3 = f.TicketFactory(tracker=tracker)
    ticket4 = f.TicketFactory(tracker=tracker)
    ticket5 = f.TicketFactory(tracker=tracker)
    db.session.commit()

    query = Ticket.query.filter(Ticket.tracker_id == tracker.id)

    def search(search_string, user=owner):
        return apply_search(query, search_string, user).all()

    assert search("") == [ticket5, ticket4, ticket3, ticket2, ticket1]
    assert search("sort:updated") == [ticket1, ticket2, ticket3, ticket4, ticket5]
    assert search("!sort:updated") == [ticket5, ticket4, ticket3, ticket2, ticket1]
    assert search("sort:id") == [ticket1, ticket2, ticket3, ticket4, ticket5]
    assert search("!sort:id") == [ticket5, ticket4, ticket3, ticket2, ticket1]

    # Changing updated timestamp changes sorting order
    ticket3.updated = datetime.utcnow()
    db.session.commit()

    assert search("") == [ticket3, ticket5, ticket4, ticket2, ticket1]
    assert search("sort:updated") == [ticket1, ticket2, ticket4, ticket5, ticket3]
    assert search("!sort:updated") == [ticket3, ticket5, ticket4, ticket2, ticket1]

    # Sort by ID remains the same
    assert search("sort:id") == [ticket1, ticket2, ticket3, ticket4, ticket5]
    assert search("!sort:id") == [ticket5, ticket4, ticket3, ticket2, ticket1]

    with pytest.raises(ValueError) as excinfo:
        search("sort:foo")

    assert str(excinfo.value) == (
        "Invalid sort value: 'foo'. "
        "Supported values are: 'id', 'updated'."
    )
diff --git a/todosrht/blueprints/tracker.py b/todosrht/blueprints/tracker.py
index 7caffdb..05af03c 100644
--- a/todosrht/blueprints/tracker.py
+++ b/todosrht/blueprints/tracker.py
@@ -90,8 +90,7 @@ def return_tracker(tracker, access, **kwargs):
    tickets = (Ticket.query
        .filter(Ticket.tracker_id == tracker.id)
        .options(subqueryload(Ticket.labels))
        .options(subqueryload(Ticket.submitter))
        .order_by(Ticket.updated.desc()))
        .options(subqueryload(Ticket.submitter)))

    try:
        terms = request.args.get("search")
diff --git a/todosrht/search.py b/todosrht/search.py
index ac75af7..1ad1068 100644
--- a/todosrht/search.py
+++ b/todosrht/search.py
@@ -62,14 +62,35 @@ def default_filter(value):
        Ticket.comments.any(TicketComment.text.ilike(f"%{value}%"))
    )

def apply_sort(query, terms, column_map):
    for term in terms:
        if term.value not in column_map:
            valid = ", ".join(f"'{c}'" for c in column_map.keys())
            raise ValueError(
                f"Invalid {term.key} value: '{term.value}'. "
                f"Supported values are: {valid}."
            )

        column = column_map[term.value]
        ordering = column.desc() if term.inverse else column.asc()
        query = query.order_by(ordering)

    return query

def apply_search(query, search_string, current_user):
    terms = list(search.parse_terms(search_string))
    sort_terms = [t for t in terms if t.key == "sort"]
    search_terms = [t for t in terms if t.key != "sort"]

    # If search does not include a status filter, show open tickets
    if not any([term.key == "status" for term in terms]):
        terms.append(search.Term("status", "open", False))
    if not any([term.key == "status" for term in search_terms]):
        search_terms.append(search.Term("status", "open", False))

    return search.apply_terms(query, terms, default_filter, key_fns={
    # Set default sort to 'updated desc' if not specified
    if not sort_terms:
        sort_terms = [search.Term("sort", "updated", True)]

    query = search.apply_terms(query, search_terms, default_filter, key_fns={
        "status": status_filter,
        "submitter": lambda v: submitter_filter(v, current_user),
        "assigned": lambda v: asignee_filter(v, current_user),
@@ -77,6 +98,11 @@ def apply_search(query, search_string, current_user):
        "no": no_filter,
    })

    return apply_sort(query, sort_terms, {
        "id": Ticket.scoped_id,
        "updated": Ticket.updated,
    })

def find_usernames(query, limit=20):
    """Given a partial username string, returns matching usernames."""
    if not query or query == '~':
-- 
2.28.0

[todo.sr.ht/patches] build success

builds.sr.ht
Details
Message ID
<C65KSEY9PQR9.31E3JXLAEOE7R@cirno2>
In-Reply-To
<20201006055422.209720-2-ivan@habunek.com> (view parent)
DKIM signature
missing
Download raw message
todo.sr.ht/patches: SUCCESS in 2m54s

[Implement ticket ordering][0] from [Ivan Habunek][1]

[0]: https://lists.sr.ht/~sircmpwn/sr.ht-dev/patches/14192
[1]: mailto:ivan@habunek.com

✓ #315153 SUCCESS todo.sr.ht/patches/debian.yml    https://builds.sr.ht/~sircmpwn/job/315153
✓ #315151 SUCCESS todo.sr.ht/patches/alpine.yml    https://builds.sr.ht/~sircmpwn/job/315151
✓ #315152 SUCCESS todo.sr.ht/patches/archlinux.yml https://builds.sr.ht/~sircmpwn/job/315152

Re: [PATCH todo.sr.ht 1/1] Implement ticket ordering

Details
Message ID
<C6CSSZHOAPSR.RT62CPI20NTA@taiga>
In-Reply-To
<20201006055422.209720-2-ivan@habunek.com> (view parent)
DKIM signature
fail
Download raw message
DKIM signature: fail
Some feedback:

- The placeholder on the search input field needs to be updated.
- We need a docs update patch as well.
- !sort:updated doesn't really make sense, how about sort:<updated and
  sort:>updated? Presume > if not specified.
Reply to thread Export thread (mbox)