Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(api) Make duration fields available as filters in the /cases/search/ page #2558

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

(api) Make duration fields available as filters in the /cases/search/ page #2558

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6b557fe
(api) Make duration fields available as filters in the /cases/search/…
mfonism Oct 15, 2021
397928c
Corrections
mfonism Oct 15, 2021
02c9cee
More corrections
mfonism Oct 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions tcms/rpc/api/testcase.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from django.db.models.functions import Coalesce
from django.forms import EmailField, ValidationError
from django.forms.models import model_to_dict
from django.utils.dateparse import parse_duration
from modernrpc.core import REQUEST_KEY, rpc_method

from tcms.core import helpers
Expand Down Expand Up @@ -279,9 +280,19 @@ def filter(query=None): # pylint: disable=redefined-builtin
:return: Serialized list of :class:`tcms.testcases.models.TestCase` objects.
:rtype: list(dict)
"""

if query is None:
query = {}

if "setup_duration" in query:
query["setup_duration"] = parse_duration(query["setup_duration"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: do comparisons (lookups __gt, __lt, etc) against values in seconds work for these fields ?

Also see for hints the __range field lookup, which is however only defined for DateTime fields:
https://docs.djangoproject.com/en/3.2/ref/models/querysets/#range

IDK if it will be any good but we may need to resort to creating our own field lookup around duration fields. I'm not seeing anything out of the box. Still, let's not get ahead of ourselves, maybe this functionality can be implemented in a more straight-forward way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, those sorts of lookups don't work for these fields in this implementation.

One way to make them work would be to transform query in this manner:

for key, val in query.items():
    if not key.startswith(
        ("setup_duration", "testing_duration", "expected_duration")
    ):
        continue

    try:
        duration = parse_duration(val)
    except TypeError:
        # val isn't a string or byte-like object
        # item is probably something like 'setup_duration__isnull=True'
        continue

    if duration is None:
        # parsing duration didn't work, leave the item (key, val) as is
        continue

    query[key] = duration

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ATM I'm not sure I like this approach. Let's agree on how the widget portion will work/look like and then we can go back to the API portion and decide what to do.


if "testing_duration" in query:
query["testing_duration"] = parse_duration(query["testing_duration"])

if "expected_duration" in query:
query["expected_duration"] = parse_duration(query["expected_duration"])

qs = (
TestCase.objects.annotate(
expected_duration=Coalesce("setup_duration", timedelta(0))
Expand Down
29 changes: 29 additions & 0 deletions tcms/rpc/tests/test_testcase.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,35 @@ def test_duration_properties_in_result(
self.assertEqual(result[0]["testing_duration"], testing_duration)
self.assertEqual(result[0]["expected_duration"], expected_duration)

def test_filter_by_setup_duration(self):
case = TestCaseFactory(setup_duration=timedelta(seconds=45))

result = self.rpc_client.TestCase.filter({"setup_duration": "0:00:45"})

self.assertIsNotNone(result)
self.assertEqual(len(result), 1)
self.assertEqual(result[0]["id"], case.pk)

def test_filter_by_testing_duration(self):
case = TestCaseFactory(testing_duration=timedelta(minutes=2))

result = self.rpc_client.TestCase.filter({"testing_duration": "0:02:00"})

self.assertIsNotNone(result)
self.assertEqual(len(result), 1)
self.assertEqual(result[0]["id"], case.pk)

def test_filter_by_expected_duration(self):
case = TestCaseFactory(
setup_duration=timedelta(seconds=45), testing_duration=timedelta(minutes=2)
)

result = self.rpc_client.TestCase.filter({"expected_duration": "0:02:45"})

self.assertIsNotNone(result)
self.assertEqual(len(result), 1)
self.assertEqual(result[0]["id"], case.pk)


class TestUpdate(APITestCase):
non_existing_username = "FakeUsername"
Expand Down
12 changes: 12 additions & 0 deletions tcms/testcases/forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,18 @@ class Meta:
widget=forms.CheckboxSelectMultiple(),
required=False,
)
setup_duration = forms.DurationField(
widget=DurationWidget(),
required=False,
)
testing_duration = forms.DurationField(
widget=DurationWidget(),
required=False,
)
expected_duration = forms.DurationField(
widget=DurationWidget(),
required=False,
)

def populate(self, product_id=None):
if product_id:
Expand Down
47 changes: 46 additions & 1 deletion tcms/testcases/static/testcases/js/search.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,21 @@ function pre_process_data (data, callback) {
})
}

function formatDuration(seconds) {
let numSecondsInDay = 24 * 60 * 60;
let days = Math.floor(seconds / numSecondsInDay);
let rest = seconds % numSecondsInDay;

let date = new Date(0);
date.setSeconds(rest);
let timeString = date.toISOString().substr(11, 8);

if (days) {
return `${days} ${timeString}`;
atodorov marked this conversation as resolved.
Show resolved Hide resolved
}
return `${timeString}`;
}
atodorov marked this conversation as resolved.
Show resolved Hide resolved

$(document).ready(function () {
const table = $('#resultsTable').DataTable({
pageLength: $('#navbar').data('defaultpagesize'),
Expand Down Expand Up @@ -106,6 +121,18 @@ $(document).ready(function () {
params.is_automated = false
};

if (!['', '0'].includes($('#id_setup_duration').val())) {
atodorov marked this conversation as resolved.
Show resolved Hide resolved
params.setup_duration = $('#id_setup_duration').val()
};

if (!['', '0'].includes($('#id_testing_duration').val())) {
params.testing_duration = $('#id_testing_duration').val()
};

if (!['', '0'].includes($('#id_expected_duration').val())) {
params.expected_duration = $('#id_expected_duration').val()
};

const text = $('#id_text').val()
if (text) {
params.text__icontains = text
Expand Down Expand Up @@ -145,7 +172,25 @@ $(document).ready(function () {
{ data: 'case_status__name' },
{ data: 'is_automated' },
{ data: 'author__username' },
{ data: 'tag_names' }
{ data: 'tag_names' },
{
data: 'setup_duration',
render: function(data, type, full, meta) {
return formatDuration(data);
}
},
{
data: 'testing_duration',
render: function(data, type, full, meta) {
return formatDuration(data);
}
},
{
data: 'expected_duration',
render: function(data, type, full, meta) {
return formatDuration(data);
}
}
atodorov marked this conversation as resolved.
Show resolved Hide resolved
],
dom: 't',
language: {
Expand Down
26 changes: 26 additions & 0 deletions tcms/testcases/templates/testcases/search.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
{% load i18n %}
{% load static %}

{% block head %}
{{ form.media }}
atodorov marked this conversation as resolved.
Show resolved Hide resolved
{% block title %}{% trans "Search test cases" %}{% endblock %}
{% endblock %}
atodorov marked this conversation as resolved.
Show resolved Hide resolved

{% block contents %}
<div class="container-fluid container-cards-pf">
Expand Down Expand Up @@ -128,6 +131,29 @@

</div>

<div class="form-group">
<label class="col-md-1 col-lg-1">{% trans "Setup duration" %}</label>
<div class="col-md-3 col-lg-3">
<div id="setup_duration">
{{ form.setup_duration }}
</div>
</div>

<label class="col-md-1 col-lg-1">{% trans "Testing duration" %}</label>
<div class="col-md-3 col-lg-3">
<div id="testing-duration">
{{ form.testing_duration }}
</div>
</div>

<label class="col-md-1 col-lg-1">{% trans "Expected duration" %}</label>
<div class="col-md-3 col-lg-3">
<div id="expected-duration">
{{ form.expected_duration }}
</div>
</div>
</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm. This should allow for an interval inclusion, e.g. search for test cases where setup_duration is between 5 and 10 minutes. See the created field on this page. With the current duration widgets the UI will become rather clunky IMO. I don't have good suggestions though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://ux.stackexchange.com/questions/45656/time-duration-entry-in-webapp-pros-cons-of-various-designs for some hints & inspiration.

The widget itself needs to be as compact as possible because of the limited real estate on the screen, however it needs to allow for selecting either an exact duration (e.g. ==) or a range (from-to). One idea that comes to mind is a more complex JS widget with popups that include the individual bootstrap duration fields. However that could also be a next step b/c it sounds like a lot of work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've looked at the SE page you linked in your comment. I have an idea for the more complex widget you suggested. I'll flesh out the idea on paper and share the sketch with you on Slack.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, let's see how it will look like. Just remember - keep it simple as much as possible.


<div class="form-group">
<div class="col-md-1 col-lg-1">
<button id="btn_search" type="submit" class="btn btn-default btn-lg">{% trans "Search" %}</button>
Expand Down