search

data-for-change / anyway

ANYWAY - Car accidents map
http://www.anyway.co.il
MIT License
77 stars 243 forks source link

Filter Widgets Data only by numeric fields #2673

Closed atalyaalon closed 3 weeks ago

atalyaalon commented 4 months ago

This is relevant for all widgets (But I think this can be done in our infra level)

Nowadays, when filtering Widgets data, we use location_info (within RequestParams). The problem is that the location_info dict ALWAYS contains both strings (like street1_hebrew or road_segment_name) and numbers (street1, road_segment_id)

Note: these strings are used both in filtering of data AND when creating various titles. We don't want to "harm" the titles functionality BUT We want to make sure to filter data ONLY by numeric fields:

Sub-urban: by road_segment_id (and road1 when necessary, for example when comparing the whole road's segments like in TopRoadSegmentsAccidentsPerKmWidget

Urban: by yishuv_symbol and street number (data is filtered both in street1 and street2 - with an "or" between them)

Current FE queries as as follows: Sub-urban: https://www.anyway.co.il/api/infographics-data?road_segment_id=900020&years_ago=5 Urban: https://www.anyway.co.il/api/infographics-data?lang=he&street1_hebrew=%D7%90%D7%91%D7%9F%20%D7%92%D7%91%D7%99%D7%A8%D7%95%D7%9C&yishuv_name=%D7%AA%D7%9C%20%D7%90%D7%91%D7%99%D7%91%20-%D7%99%D7%A4%D7%95&years_ago=5

We want to be compatible for now, meaning these queries should continue working. After creating an API query for street by yishuv_symbol and street number, I'll make sure it's implemented it in FE, and we'll be able to remove query by Hebrew names.

Note 2: After checking, street1 param from location_info is of python type <class 'sqlalchemy.util._collections.result'> and not int, perhaps this should be changed in order for this to work? (or are we taking the first value from this collection? Not sure why this is implemented in this way but I believe you'll be able to solve this one quite easily.

Note 3: I suggest that in the 2 DB tables where address exist (I think it's only markers_hebrew, involved_markers_hebrew) we'll join the street and yishuv_name using the joins in db_views.py file (I elaborated a bit more on this in the cities issue: https://github.com/data-for-change/anyway/issues/2665)

atalyaalon commented 4 months ago

@ziv17 as discussed attaching production Widgets list

ziv17 commented 4 months ago

Hi @atalyaalon, as I see it the change will not affect the results of the queries, so I do not understand the issue with compatibility with FE.

atalyaalon commented 4 months ago

@ziv17 you are right, no effect on the result. My intention was possible changing the FE query for infographics-data to be by street/city id rather than names. Nowadays it's by name). For example this one

atalyaalon commented 2 months ago

If it's all done and no other additions, we can close this one :)