gco / rietveld

Automatically exported from code.google.com/p/rietveld
Apache License 2.0
0 stars 0 forks source link

search doesn't support cc #452

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
There's an index on "cc", but the search API doesn't support it.

The index is:

- kind: Issue
  properties:
  - name: cc
  - name: closed
  - name: modified
    direction: desc

But "def search" in codereview/views.py has no URL parameter to pass that 
through.

Original issue reported on code.google.com by bradfitz@google.com on 21 Jun 2013 at 10:20

GoogleCodeExporter commented 9 years ago
I'm already working on a patch that uses App Engine Search API which makes the 
indexes obsolete.

Work in progress is here: http://codereview.appspot.com/10463043

Original comment by albrecht.andi on 22 Jun 2013 at 5:21

GoogleCodeExporter commented 9 years ago
Hopefully you don't break any existing URLs or queries?

Original comment by bradfitz@google.com on 22 Jun 2013 at 6:30

GoogleCodeExporter commented 9 years ago
No. My patch translates the URL parameters that are currently accepted into a 
search query.

Original comment by albrecht.andi on 22 Jun 2013 at 6:35

GoogleCodeExporter commented 9 years ago
What's the timeline on that going in?  It doesn't look like it supports cc yet.

Would you accept the minimal patch to support cc= with the existing search 
handler? (Not adding any new index)

Original comment by bradfitz@google.com on 24 Jun 2013 at 6:38

GoogleCodeExporter commented 9 years ago
Sure, a patch would be fine and I'd be happy to push it live.

I'm already polishing the search API patch, but don't expect it to go in before 
the weekend.

Original comment by albrecht.andi on 24 Jun 2013 at 7:01

GoogleCodeExporter commented 9 years ago
Patch at https://codereview.appspot.com/10522044/diff/1/codereview/views.py

Thanks!

Original comment by bradfitz@google.com on 24 Jun 2013 at 7:54

GoogleCodeExporter commented 9 years ago
Thanks! Fixed in revision f556795e107b.

It seems that you need cc for some automated queries. May I ping you again when 
the search API patch is ready to verify that your queries still work as 
expected?

Original comment by albrecht.andi on 25 Jun 2013 at 3:20

GoogleCodeExporter commented 9 years ago
Andi,

I have an updated patch at http://codereview.appspot.com/10553044 that fixes 
the SearchForm issue.

I've actually tested this version locally, which I should've done before. Sorry.

Could you commit that and push it live? Thanks!

Original comment by bradfitz@google.com on 25 Jun 2013 at 5:53

GoogleCodeExporter commented 9 years ago
I've submitted your patch and pushed it live, but I expect more IndexError to 
occur know.
So now I've got a challenge to finish the search API patch :)

Thanks!

Original comment by albrecht.andi on 25 Jun 2013 at 6:13