1161070 - api searches should honour the same fields in its "order" parameter as the web UI

Status: RESOLVED FIXED

(Bugzilla :: WebService, defect)

Description

[:glob ✱]

api searches should honour the same fields in its "order" parameter as the web UI.

eg. "Last Changed" --> "changeddate"

Comment 1

[David Lawrence [:dkl]]

Here is the code from buglist.cgi that the API would need to mimic

        /^Bug Number$/ && do {
            @order_columns = ("bug_id");
            last ORDER;
        };
        /^Importance$/ && do {
            @order_columns = ("priority", "bug_severity");
            last ORDER;
        };
        /^Assignee$/ && do {
            @order_columns = ("assigned_to", "bug_status", "priority",
                              "bug_id");
            last ORDER;
        };
        /^Last Changed$/ && do {
            @order_columns = ("changeddate", "bug_status", "priority",
                              "assigned_to", "bug_id");
            last ORDER;
        };

Comment 2

[David Lawrence [:dkl]]

Attachment: 1161070_1.patch

Comment 3

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8602143 [details] [diff] [review]
1161070_1.patch

Review of attachment 8602143 [details] [diff] [review]:
-----------------------------------------------------------------

r- I think using for/last and regexes that match fixed strings is pretty inefficient and hard to understand. Perhaps something like this would work?

    # Allow the use of order shortcuts similar to web UI
	
    my @order_columns;
    if ($match_params->{order}) {
        # Convert the value of the "order" form field into a list of columns
        # by which to sort the results.

        my %order_types = (
            "Bug Number"   => [ "bug_id" ],
            "Importance"   => [ "priority", "bug_severity" ],
            "Assignee"     => [ "assigned_to", "bug_status", "priority", "bug_id" ],
            "Last Changed" => [ "changeddate", "bug_status", "priority", "assigned_to", "bug_id" ],
        );
        $options{order} = $order_types{ $match_params->{order} } // [split(/\s*,\s*/, $match_params->{order})];
    }

Comment 4

[David Lawrence [:dkl]]

I am thinking now that this could be useful upstream as well.

dkl

Comment 5

[David Lawrence [:dkl]]

Attachment: 1161070_2.patch

Comment 6

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8602186 [details] [diff] [review]
1161070_2.patch

Review of attachment 8602186 [details] [diff] [review]:
-----------------------------------------------------------------

r- for the scalar-context issue with ||. Sorry :(

::: buglist.cgi
@@ +651,5 @@
> +        "Assignee"     => [ "assigned_to", "bug_status", "priority", "bug_id" ],
> +        "Last Changed" => [ "changeddate", "bug_status", "priority",
> +                            "assigned_to", "bug_id" ],
> +    );
> +    @order_columns = @{ $order_types{ $order } || [] } || split(/\s*,\s*/, $order);

subtle bug here I almost didn't notice, the left-hand-side of || is evaluated in scalar context, so the @order_columns will always be the last item in $order_types{ $order }.

Comment 7

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8602186 [details] [diff] [review]
1161070_2.patch

Woops, that was an r-

Comment 8

[David Lawrence [:dkl]]

Attachment: 1161070_3.patch

Comment 9

[David Lawrence [:dkl]]

To ssh://[e-mail]/bugzilla/bugzilla.git
   300331b..3331c1b  master -> master