search

konklone / oversight.garden

Bringing together the oversight community's work.
https://oversight.garden
Creative Commons Zero v1.0 Universal
26 stars 9 forks source link

Multiple inspector filter #108

Closed divergentdave closed 8 years ago

divergentdave commented 8 years ago

This doesn't have any UI, but the ES query object and results page are ready for searching by more than one inspector. This also includes a refactor to move query parameter parsing/defaults to one place, which will come in handy over on #90.

divergentdave commented 8 years ago

BTW this leans on querystring's handling of arrays. If you visit /reports?inspector=nasa&inspector=nsa, then the parameter comes in as an array. Similarly, if you send an array to stringify, it produces a query string with the key repeated once for each value.

konklone commented 8 years ago

Got this error when visiting http://localhost:3000/reports?inspector=nasa

TypeError: /home/eric/konklone/oversight/views/reports.html:22
   20|                     <%= results.hits.total %> reports from
   21|                     <% for (var i = 0; i < inspector.length; i++) { %>
>> 22|                       <a href="/inspectors/<%= inspector[i] %>"><%= helpers.inspector_info(inspector[i]).short_name %></a><% if (i < inspector.length - 2) { %>,<% } else if (i == inspector.length - 2) { %><% if (inspector.length > 2) { %>,<% } %> and<% } -%><% } %>.
   23|                 <% } else { %>
   24|                     <%= results.hits.total %> reports.
   25|                 <% } %>

Cannot read property 'short_name' of null
   at eval (eval at <anonymous> (/home/eric/konklone/oversight/node_modules/ejs/lib/ejs.js:464:12), <anonymous>:149:59)
   at returnedFn (/home/eric/konklone/oversight/node_modules/ejs/lib/ejs.js:493:17)
   at View.exports.renderFile [as engine] (/home/eric/konklone/oversight/node_modules/ejs/lib/ejs.js:350:31)
   at View.render (/home/eric/konklone/oversight/node_modules/express/lib/view.js:126:8)
   at tryRender (/home/eric/konklone/oversight/node_modules/express/lib/application.js:639:10)
   at EventEmitter.render (/home/eric/konklone/oversight/node_modules/express/lib/application.js:591:3)
   at ServerResponse.render (/home/eric/konklone/oversight/node_modules/express/lib/response.js:961:7)
   at /home/eric/konklone/oversight/app/routes.js:37:11
   at tryCallOne (/home/eric/konklone/oversight/node_modules/elasticsearch/node_modules/promise/lib/core.js:37:12)
   at /home/eric/konklone/oversight/node_modules/elasticsearch/node_modules/promise/lib/core.js:123:15
konklone commented 8 years ago

To be clear, http://localhost:3000/reports?inspector=nasa works on master, and shows reports only from NASA. Any ideas offhand?

divergentdave commented 8 years ago

I'm having trouble reproducing this. I did add a smoke test for this URL, and it passed https://travis-ci.org/konklone/oversight.garden/builds/123636394#L832

konklone commented 8 years ago

I can't reproduce it anymore myself, so since there tests now, the tests all pass, and you can't reproduce it either -- I'm fine going forward. Thank you!