570613 - Quick-searching doesn't return results; Firefox version is missing

Status: VERIFIED FIXED

(Socorro :: General, task)

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

This is broken on prod, too.

STR:

1. Load http://crash-stats.stage.mozilla.com/products/Firefox
2. Type a search term (anything you see as part of a stack trace, below), such as "fastcopy"
3. Press Enter/Return

Expected Results:

http://crash-stats.stage.mozilla.com/query/query?do_query=1&product=Firefox&version=Firefox%3A&query_search=signature&query_type=startswith&query=fastcopy loads and yields:

" Results within 1 weeks of 06/07/2010 16:06:15, where the crash signature starts with 'fastcopy', and the product is one of Firefox, and the version is one of Firefox: and the crashing process was a any.

No results were found."

Actual Results:

Should find results

Comment 2

[Ryan Snyder [:ryansnyder] [:rsnyder] [:rysny]]

Attachment: Patch for 570613

I got a little carried away with refactoring the Query Controller in this patch.  But it should be a lot cleaner to work in.

This patch fixes the following:

Quick Search with empty query parameter:
* will no longer returns a 500 error.
* will return results.
* will default to highlight the user's most recently selected version, if applicable.

Quick searches are now routed through /query/query instead of /query/simple for consistency.  Created the private function _simple() to update query parameters for signature searches, or forward to a specific crash report.

Also, removed the extraneous "a" from the prose statement above the search results.

This is unrelated to the problem with running advanced searches against the plugins reports tables, which was filed as a bug here - https://bugzilla.mozilla.org/show_bug.cgi?id=583311

Comment 3

[Austin King [:ozten]]

Comment on attachment 461617 [details] [diff] [review]
Patch for 570613

I'd love your take on a couple thoughts below:

routes.php isn't simple always shadowed by query/query? We should introduce a new url for simple search.

controllers/query.php - great cleanup work! I thought this logic would also be deleted from a corresponding template (views/common/list_by_signature.php or something)?

I'd prefer not to introduce state variables into the controller. Example: $this->params. Can we pass these as arguments to helper methods instead? (old _updateNavigation versus new _updateNavigation for example).

views/layout.php - Interesting. So we'll require all parameters on input? Is this to make the controller code easier? We'll still need to validate inputs, so Id rather not pollute the url with default parameters.

Comment 4

[Austin King [:ozten]]

Comment on attachment 461617 [details] [diff] [review]
Patch for 570613

Changing to r+. I was confused on routes and how _simple is used. Other feedback is a suggestion.

Comment 5

[Ryan Snyder [:ryansnyder] [:rsnyder] [:rysny]]

Thanks Austin.  I've removed the state variables and the input parameters per your suggestion.  Good call on those.

There's definitely more refactoring that can be done in the models and the views in regards to this, but this should suffice for now.  Maybe we can look into that for 2.01?

==

Sending        webapp-php/application/config/routes.php
Sending        webapp-php/application/controllers/query.php
Sending        webapp-php/application/libraries/MY_SearchReportHelper.php
Sending        webapp-php/application/views/common/prose_params.php
Sending        webapp-php/application/views/layout.php
Transmitting file data .....
Committed revision 2282.

Comment 6

[Ryan Snyder [:ryansnyder] [:rsnyder] [:rysny]]

Removing a test value.

==

Sending        webapp-php/application/controllers/query.php
Transmitting file data .
Committed revision 2283.

Comment 7

[Stephen Donner [:stephend] Not actively reading bugmail]

Hrm, is search not working, again?

http://crash-stats.stage.mozilla.com/query/query?query_type=simple&do_query=1&query=multibytetowidechar doesn't yield results, but http://crash-stats.stage.mozilla.com/report/list?range_value=2&range_unit=weeks&signature=MultiByteToWideChar&version=Firefox%3A3.6.8 does.

Comment 8

[Ryan Snyder [:ryansnyder] [:rsnyder] [:rysny]]

It looks to me like Signature is case sensitive.  Try this:
http://crash-stats.stage.mozilla.com/query/query?product=Firefox&version=ALL%3AALL&range_value=1&range_unit=weeks&date=08%2F26%2F2010+16%3A13%3A03&query_search=signature&query_type=exact&query=MultiByteToWideChar&build_id=&process_type=any&hang_type=any&do_query=1

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #8)
> It looks to me like Signature is case sensitive.  Try this:
> http://crash-stats.stage.mozilla.com/query/query?product=Firefox&version=ALL%3AALL&range_value=1&range_unit=weeks&date=08%2F26%2F2010+16%3A13%3A03&query_search=signature&query_type=exact&query=MultiByteToWideChar&build_id=&process_type=any&hang_type=any&do_query=1

Ah, indeed it is; thanks, Ryan.  Do you guys want an RFE to make it case-insensitive, or would that be too perf-intensive on the DB?

Comment 10

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on http://crash-stats.stage.mozilla.com/query/query?query_type=simple&do_query=1&query=KiFastSystemCallRet and others; we can spin off a bug if we want comment 9.