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

Always show permafrost and change missing hydrology message for areas #625

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

cstephen
Copy link
Contributor

@cstephen cstephen commented Oct 20, 2023

Closes #618.
Closes #619.

This PR:

  • Always shows the permafrost section and corresponding TOC item, even for areas and point locations with no meaningful permafrost data (or no permafrost data at all). The idea here is to always show the permafrost minimaps no matter what. However, the permafrost section is still hidden for reports that have no other data, such as Attu.
  • For area reports, instead of saying that "no data is available" for hydrology, it says that "data are not averaged over areas".

The code for hiding/showing the permafrost section has gone back and forth over the past couple years because any change to the hide/show logic in the TOC/errors/sections tends to introduce unexpected changes for less-tested locations. So, for this PR I found a ton of test locations to try various combinations of data being present/missing/effectively-missing for permafrost/hydrology and for areas/communities/points.

To test, the following is a list of the various locations I found and the expected behavior. I tested these locations as I found them, but I tweaked a couple things along the way, so I'd appreciate a second pair of eyes looking at each. Thank you!!

Hydrology + Permafrost Area Tests
Example: http://localhost:3000/report/area/19030407

  • Should show permafrost link in TOC
  • Should not include hydrology in the TOC
  • Should not show a message/error for permafrost
  • Should show a message/"error" for hydrology indicating that hydrology is not available for areas
  • Permafrost section should show MAGT maps but no chart, and explain to user why there is no chart
  • There should not be a hydrology section

Hydrology Point Tests

Community with hydrology data
Example: http://localhost:3000/report/community/AK344

  • Should show hydrology link in TOC
  • Should not show a message/error for hydrology
  • There should be a hydrology section with maps and tables

Community without hydrology data
Example: http://localhost:3000/report/community/BC203

  • Should not show hydrology link in TOC
  • Should show a message/error for hydrology
  • There should not be a hydrology section

Lat/lon with hydrology data
Example: http://localhost:3000/report/56.54/-158.66

  • Should show hydrology link in TOC
  • Should not show a message/error for hydrology
  • There should be a hydrology section with maps and tables

Lat/lon without hydrology data
Example: http://localhost:3000/report/65.15/-138.87

  • Should not show hydrology link in TOC
  • Should show a message/error for hydrology
  • There should not be a hydrology section

Permafrost Point Tests

Community with top-of-permafrost data
Example: http://localhost:3000/report/community/AK95

  • Should show permafrost link in TOC
  • Should not show a message/error for permafrost
  • Permafrost section should show MAGT maps and a top-of-permafrost chart

Community with all-zero top-of-permafrost data, but with MAGT data:
Example: http://localhost:3000/report/community/AK439

  • Should show permafrost link in TOC
  • Should not show a message/error for permafrost
  • Permafrost section should show MAGT maps but no chart, with an explanation about why there is no chart

Community without any permafrost data
Example: http://localhost:3000/report/community/AK347

  • Should show permafrost link in TOC
  • Should not show a message/error for permafrost
  • Permafrost section should show MAGT maps but no chart, and explain to user why there is no chart

Community with no data of any kind
Example: http://localhost:3000/report/community/AK26

  • Should show "We’re sorry, but this place is outside of our data..." message and nothing else

Lat/lon with top-of-permafrost data
Example: http://localhost:3000/report/64.09/-153.50

  • Should show permafrost link in TOC
  • Should not show a message/error for permafrost
  • Permafrost section should show MAGT maps and a top-of-permafrost chart

Lat/lon with all-zero top-of-permafrost data, but with MAGT data:
Example: http://localhost:3000/report/60.14/-150.19

  • Should show permafrost link in TOC
  • Should not show a message/error for permafrost
  • Permafrost section should show MAGT maps but no chart, with an explanation about why there is no chart

Lat/lon without any permafrost data
Example: http://localhost:3000/report/55.84/-155.60

  • Should show permafrost link in TOC
  • Should not show a message/error for permafrost
  • Permafrost section should show MAGT maps but no chart, and explain to user why there is no chart

Lat/lon with no data of any kind
Example: http://localhost:3000/report/51.82/-176.53

Should show "We’re sorry, but this place is outside of our data..." message and nothing else

@cstephen
Copy link
Contributor Author

It looks like all of the tests in the original PR description still apply, but here are a few new ones to test the hide-permafrost-for-Canada functionality:

For each of the locations below:

  • There should not be a TOC item for permafrost
  • There should be a "no data" message/error for permafrost
  • The permafrost section should not be visible

Communities:
http://localhost:3000/report/community/YT45
http://localhost:3000/report/community/BC579

Areas:
http://localhost:3000/report/area/BCPA859
http://localhost:3000/report/area/YTPA19
http://localhost:3000/report/area/FNTT3

Lat/lon points:
http://localhost:3000/report/64.59/-138.90
http://localhost:3000/report/57.02/-127.13

@cstephen
Copy link
Contributor Author

I experimented with incorporating an Alaska coastline GeoJSON + Turf.js into this app to help detect if a lat/lon point is Canadian or not. It worked well, but in the process of testing this I realized I was headed down a rabbit hole of over-engineering. It turns out that, because the GIPL 2.0 data covers all of Alaska, we get no benefit from pre-checking to see if a lat/lon point is within Alaska. If the point has permafrost data, it's in Alaska. If not, it's not in Alaska.

So, I've removed our earlier attempt to approximate if a lat/lon point is within Alaska using crude hard-coded lat/lon values. The new code forces the app to show the permafrost section for all areas unless the area codes are identifiably Canadian (prefixed with BCPA, FNTT, or YTPA). Otherwise, for lat/lon points and communities, the app now defers to the presence of permafrost data to determine whether to show the permafrost section. So, Canadian lat/lon points and communities will not show the permafrost section because there is no permafrost data at those locations.

I also experimented with using Turf.js to detect if polygons overlap with the Alaska coastline GeoJSON to detect purely Canadian areas that cannot be detected by area identifier alone (e.g., HUC-10 1908010100). It worked but was very slow, adding ~12 seconds of time to render every area report page. So, this type of thing is probably better handled by precompiling a list of Canadian area identifiers and checking against that list when deciding whether to show the permafrost section.

@cstephen
Copy link
Contributor Author

cstephen commented May 7, 2024

Setting this to draft mode temporarily until after our launch tomorrow. Should be good to go in theory, but requires a lot of testing to make sure it does what we need it to do for a large combination of test cases and doesn't mess anything else up, since it has the potential to affect all NCR reports.

@cstephen cstephen marked this pull request as draft May 7, 2024 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant