#11 Limit searches by time by default and setting in the search UI
Merged 6 years ago by jskladan. Opened 7 years ago by jlanda.
taskotron/ jlanda/resultsdb_frontend search-since  into  develop

@@ -10,6 +10,10 @@ 

          }

      });

  

+     var start;

+     var end;

+     var since;

+ 

      $('#testcase').typeahead({

          hint: true,

          highlight: true,
@@ -66,6 +70,59 @@ 

              $('#outcome option[value="'+item+'"]').attr('selected','selected');

          });

      }

+     if(qs.since){

+         switch(qs.since){

+             case moment().format('YYYY-MM-DD'):

+                 start = moment();

+                 end = moment();

+                 since = start.format('YYYY-MM-DD');

+                 console.log('Today');

+                 break;

+             case moment().subtract(6, 'days').format('YYYY-MM-DD'):

+                 start = moment().subtract(6, 'days');

+                 end = moment();

+                 since = start.format('YYYY-MM-DD');

+                 console.log('Last 7 Days');

+                 break;

+             case moment().subtract(29, 'days').format('YYYY-MM-DD'):

+                 start = moment().subtract(29, 'days');

+                 end = moment();

+                 since = start.format('YYYY-MM-DD');

+                 break;

+             default:

+                 dates = qs.since.split(',');

+                 start = moment(dates[0], 'YYYY-MM-DD');

+                 if(dates.length > 0)

+                 {

+                     end = moment(dates[1], 'YYYY-MM-DD').subtract(1, 'days');

+                 }

+                 since = qs.since;

+                 break;

+         }

+     }else{

+         start = moment().subtract(29, 'days');

+         end = moment();

+         since = start.format('YYYY-MM-DD') + ',' + end.format('YYYY-MM-DD');

+     }

+ 

+     $("#searchrange").daterangepicker({

+         startDate: start,

+         endDate: end,

+         ranges: {

+            'Today': [moment(), moment()],

+            'Last 7 Days': [moment().subtract(6, 'days'), moment()],

+            'Last 30 Days': [moment().subtract(29, 'days'), moment()],

+         }

+     }, function(start, end, label) {

+         if (label === 'Custom Range'){

+             //$('#searchrange').val(start.format('MMMM D, YYYY') + ' - ' + end.format('MMMM D, YYYY'));

+             since = start.format('YYYY-MM-DD') + ',' + end.add(1, 'days').format('YYYY-MM-DD');

+         }

+         else{

+             $('#searchrange').val(label);

+             since = start.format('YYYY-MM-DD');

+         }

+     })

  

      // Replace the submit button behaviour

      $("#searchform").submit(function(e){
@@ -120,6 +177,8 @@ 

          }

          if(outcome)

              url += "&outcome="+outcome;

+         

+         url += "&since="+since;

  

          console.log(url);

          window.location.href = encodeURI(url);

@@ -5,6 +5,7 @@ 

        <meta name="viewport" content="width=device-width, initial-scale=1.0">

        <!-- Bootstrap -->

        <link rel="stylesheet" href="//netdna.bootstrapcdn.com/bootstrap/3.0.0/css/bootstrap.min.css">

+       <link rel="stylesheet" type="text/css" href="https://cdn.jsdelivr.net/npm/daterangepicker/daterangepicker.css">

        <link rel="stylesheet" type="text/css" href="{{ url_for('static', filename='css/style.css') }}">

    </head>

  
@@ -63,6 +64,10 @@ 

                </select>

              </div>

  

+             <div class="form-group">

+               <input class="form-control" id="searchrange" value=""></input>

+             </div>

+ 

              <button type="submit" class="btn btn-default" id="go">Go!</button>

            </form>

          </div>
@@ -94,6 +99,8 @@ 

      <script src="//netdna.bootstrapcdn.com/bootstrap/3.0.0/js/bootstrap.min.js"></script>

      <!-- Search logic -->

      <script src="https://cdnjs.cloudflare.com/ajax/libs/typeahead.js/0.11.1/typeahead.bundle.min.js"></script>

+     <script src="https://cdnjs.cloudflare.com/ajax/libs/moment.js/2.22.2/moment.min.js"></script>

+     <script type="text/javascript" src="https://cdn.jsdelivr.net/npm/daterangepicker/daterangepicker.min.js"></script>"

      <script src="{{ url_for('static', filename='js/search.js') }}"></script>

      <div id="testcaseEndpointURL" style="display: none;">{{url_for('main.testcase_tokenizer')}}</div>

  

Limit search by default to last 30 days and add a date range picker to search form

This addresses #4 . @lbrabec or @jskladan , can you please review this?

Metadata Update from @kparal:
- Request assigned

7 years ago

Looks reasonable to me, but I only briefly checked the code on my phone (sti=
ll on vacation...), so do not treat this as a real review (somebody has to r=
un and try it dor reals...). But the concept is sound.=

Sorry for dropping the ball on this.

Today I got to try it, and sadly, the way you programmed it does not really work. I suggest you try it yourself - either by spinning up your instance of resultsdb (backend) or using the public resultsdb-dev data (to do that create a conf/settings.py file in your local checkout, containing RDB_URL = 'http://taskotron-dev.fedoraproject.org/resultsdb_api/api/v2.0' line).

The problem with the datepicker, as you implemented it is that it sets the since= URL parameter with both start and end values, if i take the 'today' option as an example, the url parameter is set to since=2018-09-26,2018-09-26, these dates are (internally in resultsdb backend) exploded to 2018-09-26 00:00:00.00, and thus effectively limiting the shown results to none (as the probability of a result being created precisely at midnight to a mili/nanosecond is quite low), instead of showing the results from 'today'.
Exactly the same happens for 'tomorrow' or when you custom-select any single day.

In multi-day ranges, the "hour expansion" effectively hides all the results from the day at the end of the interval (last-x-days options of any custom range).

Note, that the since= paramter is quite smart, so the way I'd like this done is:
- limit the options to: today, last 7 days, last 30 days, custom range
- for the first three, use the since=start_date format (without the end_date part), so this limits the shown results to "all that were created at the midnight on start_date and newer"
- for the 'custom range' - either drop the option, or make sure that the end_date bit is set to the next day after the selected interval (thus showing the results from the last day of the interval)

Also, the search form is intentionaly smart, see for yourself, that if you use any (other than the date filtering you just added) filter, the fields are set in at the "filtered" page. The date filtering you implemented should also have this feature, to keep it consistent.

Thanks!

Oki doki.

What should be the default value for the picker? 30 days as suggested by @kparal on #4 ?

Regards

@jlanda last 30 days seems reasonable to me

rebased onto 901e551

6 years ago

Updated PR with changes.

I have some questions. Now, althrought the filter is set to 30 days by default, the initial query is not filtered. Should I change this? In case I have to change the default query, should I disallow to use raw /results without any date range or should I keep the raw /results behaviour but redirect user to /results&since=whatever as default url when they enter / ?

Nope, let the default be as it is. Thing is, the default query is limited to top X (20 IIRC) results in the DB, which is the fastest query there is. The time constraint is important for the more "specific" queries, because these can/tend to be scattered through the dataset, so the 30-day default is there to limit the scope of the database the query needs to traverse before returning results. For example, imagine there are only 9 results for the specific query in the whole dataset, but you want to get the first 10 or 20, then the whole history of the data needs to be traversed, to be sure there are no more than 9.

So please do not change any other behaviour than what's defined by the ticket's scope. Thanks!

ready for review then ;)

Much better now, but the whole switch statement is IMO unnecessary, and a bit heavy handed.

First of all, in each case the start assignment can be easily replaced with the two lines of the default section. The rest of the code in each section is the same.

Secondly, the default section does not handle all the "other" cases properly - try accessing an url with ...&since=2018-10-01 in it (for example, in practice any single-date since other than coincidental today/past [7,30] days does the trick), and observe the Invalid date at the second position in the datepicker. Also, the if-clause in the default section IMO has a typo in the condition (aka - if qs.since is non-empty, the clause is always true, and unnecessary as such).

I'll be replacing the whole switch block with this::

    dates = qs.since.split(',');
    start = moment(dates[0], 'YYYY-MM-DD');
    end = moment();
    if (dates.length > 1){
        end = moment(dates[1], 'YYYY-MM-DD').subtract(1, 'days');
    }
    since = qs.since;

Merged by a8dde23 and 13cdad0

Thank you, @jlanda for the great effort!

Pull-Request has been closed by jskladan

6 years ago

Commit 13cdad0 fixes this pull-request

Pull-Request has been merged by jskladan

6 years ago