Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter Widgets Data only by numeric fields #2673

Closed
atalyaalon opened this issue Jun 24, 2024 · 4 comments
Closed

Filter Widgets Data only by numeric fields #2673

atalyaalon opened this issue Jun 24, 2024 · 4 comments
Assignees
Milestone

Comments

@atalyaalon
Copy link
Collaborator

atalyaalon commented Jun 24, 2024

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: #2665)

@atalyaalon atalyaalon added this to the v.0.17.0 milestone Jun 24, 2024
@atalyaalon
Copy link
Collaborator Author

@ziv17 as discussed attaching production Widgets list

@ziv17
Copy link
Collaborator

ziv17 commented Jul 11, 2024

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
Copy link
Collaborator Author

@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
Copy link
Collaborator Author

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

@ziv17 ziv17 closed this as completed Oct 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants