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

Adds Geospatial custom fields and Map/Attribute viewer to Record view #221

Open
wants to merge 16 commits into
base: geospatial
Choose a base branch
from

Conversation

spilth
Copy link
Contributor

@spilth spilth commented Dec 11, 2024

This is the beginning of adding Geospatial functionality to UltraViolet.

This PR includes the following functionality/features:

  • Adds custom fields for GeoServer layer name, WMS/WFS layer availability and bounding box
  • Adds a Leaflet Map, attributes list and web services detail views for geospatial records
  • Adds the ability for users to click feature to see their attributes
  • Adds a GeoServer proxy for WFS requests (attributes list and feature details)
  • Prevents anonymous users from seeing map, attributes and web service details for restricted records
  • Adds view tests for the above

Addresses #209, #210, #211 and #212

Note: I'd prefer to squash this before merging it, so I'd like to merge it myself once it's approved.

Screenshots

Custom Fields Editing

Screenshot 2024-12-11 at 3 50 09 PM

Restricted Record

Screenshot 2024-12-11 at 3 47 39 PM

Public Record

Screenshot 2024-12-11 at 3 48 16 PM

@spilth spilth requested a review from ekate December 11, 2024 21:50
Comment on lines +78 to +80
"https://*.basemaps.cartocdn.com",
"https://maps-public.geo.nyu.edu",
"https://maps-restricted.geo.nyu.edu",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are here to allow pulling content from external sources on the front-end.

@@ -421,6 +429,8 @@ if APP_ENVIRONMENT == 'local':
'[email protected]': 'adminUV'
}

RATELIMIT_ENABLED = False
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only enabled in local and to allow bulk loading data via the REST API.

Copy link
Contributor

@ekate ekate left a comment

Choose a reason for hiding this comment

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

We might ask somebody else to look at javascript/css part, everything else looks good to me.

@spilth spilth requested review from lhenze and dismorfo December 12, 2024 14:13
Copy link
Contributor

@dismorfo dismorfo left a comment

Choose a reason for hiding this comment

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

Hi there. The code looks good. Since we don't have a linter rules file or a document outlining best practices, I will keep it short.

Error Handling: The fetch call log errors to the console instead of using user-friendly messages or retry mechanisms. Try contacting @lhenze and asking her about UI expectations when fetch fails.

Comments: Add comments to the code. This is a suggestion since we don't have a document describing how to document our work.

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