okfn-brasil / jarbas

🎩 API for information and suspicions about reimbursements by Brazilian congresspeople
https://jarbas.serenata.ai/
296 stars 61 forks source link

The 'yes'/'no' dashboard filters #256

Closed giovanisleite closed 7 years ago

giovanisleite commented 7 years ago

Hi,

I was looking the pull request #254 (implementing the has_receipt_url filter that is like the suspicions), and now I am confuse about the operation of the filter.

How should these filters work? e.g. "suspicions" "All": shows all the reimbursements; "Yes": Show only reimbursements that are suspect; "No": Show only reimbursements that aren't suspect. Right?

So... why there isn't a query for the 'no'? Like return self.exclude(suspicions__isnull=True) ? for the 'no', returns just the queryset that is the same return for 'all' (ausence of value).

If I am right about this, there is a bug on the 'no' filter

Like this, using another filters just too see the number of results. Filters that I used: Month: December, Year: 2016. State: SP. Subquota: 'Congressperson meal' (id 13).

Without suspicious filter (selected: 'All'): 259 results. When I try 'yes' for suspicious, 1 result. When I try 'no' for suspicious, 259 results. I think that isn't working, because should be 258 results (1 suspect + 258 that isn't suspect).

Am I missing something?

cuducos commented 7 years ago

That makes a lot of sense… you've found a bug , yay!

My only comment would be self.filter(suspicions=None) instead of self.exclude(suspicions__isnull=True) ; )

giovanisleite commented 7 years ago

you've found a bug , yay!

:beetle: hahaha

My only comment would be self.filter(suspicions=None) instead of self.exclude(suspicions__isnull=True) ; )

Why? The query seems equal

cuducos commented 7 years ago

Why? The query seems equal

They are not equal, they produce equal results.

>>> import this
The Zen of Python, by Tim Peters

Beautiful is better than ugly.
Explicit is better than implicit.
Simple is better than complex.
[…]
Readability counts.

A double negative resolves to a positive (excluding what is not None is filtering only what is None). But double negative arguably are confusing — i.e. less implicit (and explicit is better than implicit), less simple (and simple is better than complex), more difficult to read (and readability counts) and IMHO less beautiful (and beautiful is better than ugly) ; )

giovanisleite commented 7 years ago

I was looking for performance problems and checked the sql query Now I see the problem, Thanks @cuducos

I will do it =)