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 numeric only fields in Widget queries #2686

Merged
merged 3 commits into from
Sep 7, 2024

Conversation

ziv17
Copy link
Collaborator

@ziv17 ziv17 commented Aug 15, 2024

Closing #2673

@codecov-commenter
Copy link

codecov-commenter commented Aug 15, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 80.00000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 52.63%. Comparing base (400af13) to head (ebb68ce).
Report is 93 commits behind head on dev.

Files with missing lines Patch % Lines
...dgets/injured_accidents_with_pedestrians_widget.py 33.33% 2 Missing ⚠️
...ay/widgets/urban_widgets/urban_crosswalk_widget.py 33.33% 2 Missing ⚠️
anyway/widgets/widget_utils.py 90.47% 2 Missing ⚠️
anyway/models.py 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #2686      +/-   ##
==========================================
- Coverage   53.22%   52.63%   -0.59%     
==========================================
  Files         119      122       +3     
  Lines        9924    10153     +229     
==========================================
+ Hits         5282     5344      +62     
- Misses       4642     4809     +167     
Flag Coverage Δ
unittests 52.63% <80.00%> (-0.59%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ziv17 ziv17 force-pushed the filter-numeric-only-fields branch from f9b77c9 to 69f9571 Compare August 15, 2024 14:45
@ziv17 ziv17 marked this pull request as ready for review August 15, 2024 14:45
@ziv17 ziv17 requested review from atalyaalon and tkalir August 15, 2024 14:46
@atalyaalon
Copy link
Collaborator

atalyaalon commented Sep 4, 2024

@ziv17 Not sure if necessary for this pr, but note that in news_flash table, for some reason yishuv_symbol is null even when yishuv_name exist, I suggest changing this and making sure we insert yishuv_symbol is inserted to news_flash table (and perhaps backfilling it).
Same for street1 and street2

@atalyaalon
Copy link
Collaborator

@ziv17 looks great! I
I added a small commit of a bug (not related to pr).
I want to merge but before that I need your response to the two comments, to make sure there are no bugs there

@ziv17 ziv17 force-pushed the filter-numeric-only-fields branch from cf978e0 to 2a5b5bb Compare September 7, 2024 16:39
@atalyaalon atalyaalon merged commit 48a7863 into data-for-change:dev Sep 7, 2024
3 checks passed
@atalyaalon
Copy link
Collaborator

@ziv17 thanks for the fix! merged :)

@ziv17 ziv17 deleted the filter-numeric-only-fields branch September 7, 2024 16:50
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

Successfully merging this pull request may close these issues.

3 participants