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.

Sign up for GitHub

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

Jump to bottom

Datahub: handle filter geometry error #700

Merged
merged 4 commits into from
Nov 29, 2023
Merged

Datahub: handle filter geometry error #700

merged 4 commits into from
Nov 29, 2023

Conversation

fgravin
Copy link
Member

@fgravin fgravin commented Nov 21, 2023

#694

The validation can

  • throw an error is the GeoJson is malformed
  • return false if the GeoJson is not valid.

Both cases are supported.
This ensure the search runs even with a wrong geometry filter.

I'll provide test if you approve the approach.

@fgravin fgravin added the bug Something isn't working label Nov 21, 2023
@fgravin fgravin added this to the 2.1.0 milestone Nov 21, 2023
@fgravin fgravin requested a review from jahow November 21, 2023 21:17
@fgravin fgravin linked an issue Nov 21, 2023 that may be closed by this pull request
Datahub: geometry boost error if area of competence is empty #694
Closed
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 21, 2023 •
edited
Loading

Affected libs: feature-search, feature-router, feature-map, feature-dataviz, feature-record, data-access-datafeeder, api-metadata-converter, api-repository, feature-catalog, feature-auth, feature-editor, ui-search, util-data-fetcher, data-access-gn4, util-app-config, common-fixtures, ui-elements, ui-catalog, util-shared, ui-widgets, ui-inputs, ui-map, common-domain, ui-dataviz, util-i18n, ui-layout,
Affected apps: metadata-editor, datahub, demo, webcomponents, search, map-viewer, metadata-converter, datafeeder, data-platform,

  • 🚀 Build and deploy storybook and demo on GitHub Pages
  • 📦 Build and push affected docker images

@fgravin
Copy link
Member Author

fgravin commented Nov 25, 2023

@jahow could you check this one too ?
Thanks

jahow
jahow reviewed Nov 29, 2023
View reviewed changes
Copy link
Collaborator

@jahow jahow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments on how to handle the invalid geom case, thanks!

libs/feature/search/src/lib/state/effects.ts Outdated Show resolved Hide resolved
libs/feature/search/src/lib/state/effects.ts Outdated Show resolved Hide resolved
@fgravin
chore: add geojson-validation package
358f443 
in order to valide geojson, for instance when used in ES search payload
@fgravin
fix(search): handle filter geometry issue
d8f4aa4 
ATM if the geometry is not valid, the search crashes as it's passer the ES query. This ensure the geometry is valid, and use null if not
@fgravin
fix(geom-filter): warn user only if the geometry if not valid
650eab0 
while previous code was warning even if the geometry service returns an error
@coveralls
Copy link

coveralls commented Nov 29, 2023 •
edited
Loading

Coverage Status

coverage: 83.812% (-2.3%) from 86.117%
when pulling 69c9d3f on empty-geometry
into 998c2e2 on main.

@fgravin fgravin requested a review from jahow November 29, 2023 17:19
@fgravin fgravin merged commit 3351a2d into main Nov 29, 2023
7 of 8 checks passed
@fgravin fgravin deleted the empty-geometry branch November 29, 2023 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jahow jahow Awaiting requested review from jahow

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
2.1.0
Development

Successfully merging this pull request may close these issues.

Datahub: geometry boost error if area of competence is empty
3 participants
@fgravin @coveralls @jahow