[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Health-dev] Group Access Permission Rule: How to filter by Patient

From: Chris
Subject: Re: [Health-dev] Group Access Permission Rule: How to filter by Patient Status = Hospitalized
Date: Fri, 12 Jun 2015 14:01:56 -0700
User-agent: Mutt/1.5.23 (2014-03-12)

Thanks Cédric!

I was thinking of modifying the get_patient function since you said not
to use raw sql queries. Now, I will. Hehe.

> I think it is better to not have silent failure but instead adding just
> a test on the operator and value like this:
>     _, operator, value = clause
>     if operator not in ['=', '!=']:
>         raise ValueError('Wrong operator: %s' % operator)
>     if value is not True and value is not False:
>         raise ValueError('Wrong value: %s' % value)
> Also the code return wrong result if '!=' is used.

I didn't even think about that! I'll update the code.

> Now, I have also looked at the 'get_patient_status' method despite it
> uses raw SQL instead of python-sql query. It is really not optimal
> because it generates a query per record. It should be a classmethod
> searcher with a query that should look like this:
>     @classmethod
>     def get_patient_status(cls, ids, name):
>         pool = Pool()
>         Registration = pool.get('gnuhealth.inpatient.registration')
>         registration = Registration.__table__()
>         result = dict.fromkeys(ids, False)
>         for sub_ids in grouped_slice(ids):
>             clause_ids = reduce_ids(, sub_ids)
>             query =, Literal(True),
>                 where=(registration.state == 'hospitalized') & clause_ids,
>                 group_by=registration.patient)
>             cursor.execute(*query)
>             result.update(cursor.dictfetchall())
>         return result

Ahh, that makes sense. I'll look for other instances and try to emulate
that design.

Thanks again!


PS: I think you are right and we probably should have some sort of code
review tool. I suppose we should add a task for that, too.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]