onaio / fhir-gateway-extension

This repo holds the OpenSRP permissions checker and data access checker
Other
0 stars 1 forks source link

[Bug Report] Using the `administrativeLevelMax` and `administrativeLevelMin` filters does not seem work #75

Closed dubdabasoduba closed 2 months ago

dubdabasoduba commented 2 months ago

Context

Implementation

lincmba commented 2 months ago

@dubdabasoduba @ndegwamartin I have done some research and found the following. There is an edge case that we didn't handle in the implementation of the administrative levels filters. The LocationHierarchy endpoint works by first getting the details of the location provided, then using it as a parent location to get all child locations in the next level of the hierarchy and then using these children as parents to get the next level, and so on, until the lowest hierarchy. Also note that the administrative level increases as you go down the location hierarchy.

Here's the edge case: say the parent location is of administrative level 4, and the children range from level 5 to 10. If we put an administrative filter of 6 to 10 (administrativeLevelMin=6&administrativeLevelMax=10), the hierarchy won't return any child, just the parent. This is because when the location hierarchy tries to fetch the children of the parent of level 4, the next level of children are of level 5, and that is not within the boundaries set of 6 to 10. Hence, there won't be any further hierarchy built from this.

To fix this, we must adjust the implementation of administrativeLevelMin. Instead of using the user-provided administrativeLevelMin parameter directly, we should set administrativeLevelMin to the value of the parent administrative level. This way, we ensure that all children within the range of the parent's level are fetched correctly.

This also raises the question: is there any need to have the user provide administrativeLevelMin parameter?

ndegwamartin commented 2 months ago

@lincmba given the example above doesn't it mean the parent is now administrativeLevel 6? and thus we expect data for levels 6 through 10.

My assumption is that all levels are numerically ordered i.e. root to leaf is incrementing

lincmba commented 2 months ago

@lincmba given the example above doesn't it mean the parent is now administrativeLevel 6? and thus we expect data for levels 6 through 10.

My assumption is that all levels are numerically ordered i.e. root to leaf is incrementing

@ndegwamartin The parent location does not change its administrative level based on the filter parameters. The administrative levels are indeed numerically ordered from root to leaf, incrementing as you go down the hierarchy.

To clarify, the administrative level of a location is a fixed attribute and does not change dynamically based on query parameters. The administrative level of a location is assigned at the time of creation. For example, if a location is created as a child of a location with level 3, it will be assigned level 4. When querying the LocationHierarchy endpoint, the administrative levels of locations remain unchanged. Filters such as administrativeLevelMin and administrativeLevelMax are used to determine which levels within the hierarchy to include in the response.

In the example above: The administrative level of the parent location is fixed at level 4 and does not change to level 6 based on the filter parameters. The children levels (5 to 10) remain the same as well. The filter parameters (administrativeLevelMin and administrativeLevelMax) are used solely to define the range of levels to include in the query response, not to change the levels of existing locations.

ndegwamartin commented 2 months ago

I see. I think it makes sense to not show any children whose locations do not fall under the filter. We can probably fix it by adding a check such that, if the requested location level is less than the administrative min, then make the min the parent to start fetching from. The user should understand that the min and max filters are used to restrict what gets returned regardless of the location provided. The location may only be used to ensure the lineage for the filtered locations i.e. return only locations within min and max admin levels that ware descendants of the requested location.

This is what I'm thinking:

Kenya - Level 1 Counties - Kitui, Mombasa - Level 2 Constituency - Kitui East, Nyali - Level 3 Ward - Kilimambogo, Kongowea - Level 4 Village - Kilimambogo Village, Kongowea Village - Level 5

So assuming the query is GET /LocationHierarchy?locations=Mombasa&administrativeLevelMin=4&administrativeLevelMax=5

I'd expect the algorithm to return Kongowea and Kongowea villages only

ndegwamartin commented 2 months ago

And since the algorithm is sequential the easiest approach might be to start the processing from the provided location , Mombasa in the above example skip adding all levels that are not in the bounds till you get to the min admin level.