GitHub PullRequestReviewCommentEvent at 2014-04-30 17:47:46
Repository: commcare-hq
Actor: esoergel
Payload: {"comment":{"url":"https://api.github.com/repos/dimagi/commcare-hq/pulls/comments/12155991","id":12155991,"diff_hunk":"@@ -107,6 +107,42 @@ class BaseHNBCReport(CustomProjectReport, CaseListReport):\n     def case_es(self):\n         return ReportCaseES(self.domain)\n \n+    def build_es_query(self, case_type=None, afilter=None, status=None):\n+\n+        def _domain_term():\n+            return {\"term\": {\"domain.exact\": self.domain}}\n+\n+        subterms = [_domain_term(), afilter] if afilter else [_domain_term()]\n+        if case_type:\n+            subterms.append({\"term\": {\"type.exact\": case_type}})\n+\n+        if status:\n+            subterms.append({\"term\": {\"closed\": (status == 'closed')}})\n+\n+        query_block = {\"match_all\": {}}\n+\n+        and_block = {'and': subterms} if subterms else {}","path":"custom/apps/crs_reports/reports.py","position":18,"original_position":18,"commit_id":"6ec273553a18471b6f33d3011155f02f4a3e6a35","original_commit_id":"6ec273553a18471b6f33d3011155f02f4a3e6a35","user":{"login":"esoergel","id":2367539,"avatar_url":"https://avatars.githubusercontent.com/u/2367539?","gravatar_id":"3ffda11b7953b849e1d97fd2c17f0e59","url":"https://api.github.com/users/esoergel","html_url":"https://github.com/esoergel","followers_url":"https://api.github.com/users/esoergel/followers","following_url":"https://api.github.com/users/esoergel/following{/other_user}","gists_url":"https://api.github.com/users/esoergel/gists{/gist_id}","starred_url":"https://api.github.com/users/esoergel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/esoergel/subscriptions","organizations_url":"https://api.github.com/users/esoergel/orgs","repos_url":"https://api.github.com/users/esoergel/repos","events_url":"https://api.github.com/users/esoergel/events{/privacy}","received_events_url":"https://api.github.com/users/esoergel/received_events","type":"User","site_admin":false},"body":"If subterms is empty, this will fall back to `\"filter\": {}`, which will match nothing.  I think you want `\"filter\": {\"match_all\": {}}` in this case.  At any rate, subterms cannot be empty, given the way it is constructed above it must have at least the domain term (rightly so).  You can probably just remove the `if subterms else {}` bit.  Otherwise this looks good to go.","created_at":"2014-04-30T17:47:46Z","updated_at":"2014-04-30T17:47:46Z","html_url":"https://github.com/dimagi/commcare-hq/pull/3295#discussion_r12155991","pull_request_url":"https://api.github.com/repos/dimagi/commcare-hq/pulls/3295","_links":{"self":{"href":"https://api.github.com/repos/dimagi/commcare-hq/pulls/comments/12155991"},"html":{"href":"https://github.com/dimagi/commcare-hq/pull/3295#discussion_r12155991"},"pull_request":{"href":"https://api.github.com/repos/dimagi/commcare-hq/pulls/3295"}}}}