Issue 889: Add Observation Only Zones #904

wheninseattle · 2025-02-26T05:30:08Z

Description

This feature incorporates observation only zones into Avy; applying observation zone names to the observation preview cards and adding these zones to the filter widget.

Details

Some centers (MWAC, BTAC, others?) have designated observation only zones outside of their official forecast zones. When available, a url to a KML file is provided in the avalanche center's meta data at: >> metaData.widget_config.observation_viewer.alternate_zones
This url is passed to the useAlternateObserationZones hook which fetches and parses the KML file, returning features similar to those in the mapLayer.
If provided, these alternate zones are merged with the features from the mapLayer prior to the flattening of the observations list, so that these zones labels are properly associated with the observations.
Optional This mergedMapLayer is subsequently filtered by the available observations, removing zones that don't contain any observations. I could see this being a positive to reduce clutter in the already full filter widget, but on the other hand it could be advantageous for users to see the full list of zones even if they don't have observations.
Finally, the resulting, filteredMapLayer is passed to the ObservationsFilterForm.

Related Issue

Closes #889

Acceptance Criteria

When available, add observation only zones to observation list view and filter
Observation preview cards, should be labeled with appropriate zone name, including observation only zones
When observation only zone is selected in the ObservationFilterForm, the list of observations should be correctly filtered for that zone.

Considerations

The map view on the MWAC and BTAC websites does not show the observation only zones, so I decided to decouple the mapLayer and the observation only zones to match this. Additionally, merging them into the mapLayer was breaking the map view.
In the ObservationsFilterForm, do we want to visually separate "{Center Id} Forecast Zones" and "Other Regions" similar to the website form or create collapsible sections? If there are too many zones that push other sections off screen, will those other sections go unnoticed or be difficult to navigate to?
As mentioned in "details", do we want to filter out zones that have no observations?
Currently "unknown zone" still flashing briefly when observation preview cards initially render.
As you can tell, Typescript is not my daily driver; I'm certainly open to any suggestions for how I can improve my typing!

Updates

Before

After

Testing Steps / QA Criteria

Use git pull wn-889-add-observation-zones
yarn start
Set avalanche center to BTAC or MWAC
Check to ensure that observation preview cards are labeled with alternate zones when appropriate.
Check to ensure that the ObservationsFilterForm includes additional, observation only zones

…tions

components/observations/ObservationsFilterForm.tsx

stevekuznetsov · 2025-02-26T13:42:24Z

components/observations/ObservationsFilterForm.tsx

                    </View>
                  )}
                  {mapLayer && (
+                    // TODO: Render in two groups with respective labels: <Center_ID> Forecast Zones & Other Regions


@rustynwac let's pin down the UX we want for the two sets of zones in the filter modal with an eye to what the websites are doing.

@rustynwac @stevekuznetsov did we decide how we want to represent the zones?

@rustynwac let's sync on this

components/observations/ObservationsListView.tsx

stevekuznetsov · 2025-02-26T13:44:05Z

components/observations/ObservationsListView.tsx

  const [filterModalVisible, {set: setFilterModalVisible, on: showFilterModal}] = useToggle(false);
+  const {data: metaData} = useAvalancheCenterMetadata(center_id);
+  const alternateZonesUrl = metaData?.widget_config?.observation_viewer?.alternate_zones || '';
+  const {data: alternateZones} = useAlternateObservationZones(alternateZonesUrl, {


Please keep the result pattern and add this to the incompleteQueryState block.

Will do - that incompleteQueryState block is pretty sweet!

stevekuznetsov · 2025-02-26T13:45:43Z

components/observations/ObservationsListView.tsx

  );

-  if (incompleteQueryState(observationsResult, mapResult) || !mapLayer) {
+  if (incompleteQueryState(observationsResult, mapResult) || !filteredMapLayer) {


The check for !mapLayer is to guard against that request returning nothing - let's check here for the responses, not computed values from the responses, e.g. || !mapLayer || !centerMetadata || !alternateZones

That makes sense - thanks

components/observations/ObservationsListView.tsx

stevekuznetsov · 2025-02-26T13:49:32Z

hooks/useAlternateObservationZones.ts

+  coordinates: string;
+}
+
+interface KMLPlacemark {


Please write a zod schema (or generate one from the interface here, or a known good KML library like togeojson) so we can parse and validate our incoming data.

hooks/useAlternateObservationZones.ts

stevekuznetsov · 2025-02-26T13:52:03Z

hooks/useAlternateObservationZones.ts

+
+export const useAlternateObservationZones = (url?: string, options?: UseAlternateObservationZonesOptions): UseQueryResult<AlternateObservationZones[] | null, AxiosError> => {
+  const {logger} = useContext<LoggerProps>(LoggerContext);
+  const key = ['alternateObservationZone-', url];


Could we please mirror the pattern in other hooks to allow for pre-fetching?

hooks/useAlternateObservationZones.ts

stevekuznetsov · 2025-02-26T13:53:42Z

hooks/useAlternateObservationZones.ts

+
+  const alternateZones: AlternateObservationZones[] = kmlData.kml.Document.Folder.Placemark.map(placemark => {
+    let coordinates: [number, number][] = [];
+    const parseCoordinates = (coordinateString: string): [number, number][] => {


This is a good candidate for a set of unit tests!

@stevekuznetsov are you speaking specifically about the coordinate parsing function or the broader KML parsing functionality?

Any of the logic here which takes static data as input and spits out other static data in some other format!

@stevekuznetsov I added a few unit tests for this. It looks like a recent package change may have caused a version conflict for "strip-ansi", causing the Jest tests to fail with "TypeError: stripAnsi is not a function". It looks like a dependency of "xml-js" may have changed the "strip-ansi" version.

I'm curious about the trade offs of breaking out smaller functions like parseCoordinates for testing. On the one hand this exposes them for individual unit test, but on the other they no longer have access to objects in the parent function (like logger).

stevekuznetsov · 2025-02-26T13:55:40Z

components/observations/ObservationsListView.tsx

+      );
+      return {
+        ...mapLayer,
+        features: [...mapLayer.features, ...zonesNotInMapLayer],


When we display observations to the user in card form, are we grabbing the first item from this list that matches for labelling? I think we should try to be explicit - do users expect to see an observation matching a custom zone and a forecast zone labelled with the former or the latter?

@stevekuznetsov, sorry, I'm not entirely sure what you mean here. Are you saying that we want to make sure that if there are multiple matches, we want to be explicit about prioritizing the forecast zone?

I'm actually not sure what avalanche center staff would expect in this situation - I've reached out to the NAC and I'll follow up here once I hear back. However, regardless of which path is taken, I was hoping we could have explicit logic (observation, forecast_zones, custom_zones) => label that would encode the choice, as opposed to having implicit logic (return the first match from a list containing both types of zones that might or might not be sorted in a specific order).

Got some clarity from the NAC, we should prioritize the custom zones if present.

Thanks for getting clarification on that!

wheninseattle · 2025-03-18T00:19:48Z

components/observations/ObservationsListView.tsx

        <ObservationsFilterForm
          requestedTime={requestedTime}
-          mapLayer={mapLayer}
+          mapLayer={filteredMapLayer}


@stevekuznetsov I initially had a type for alternate observation zones, however, this creates type issues when those zones are merged with other zones from the map layer. Would there be future utility for adding the alternate zones to the actual map layer, or should I change the type expected for mapLayer here?

wheninseattle · 2025-04-04T19:29:05Z

components/observations/ObservationsFilterForm.tsx

                    </View>
                  )}
                  {mapLayer && (
+                    // TODO: Render in two groups with respective labels: <Center_ID> Forecast Zones & Other Regions


@rustynwac @stevekuznetsov did we decide how we want to represent the zones?

wheninseattle · 2025-04-04T19:45:15Z

hooks/useAlternateObservationZones.ts

+
+  const alternateZones: AlternateObservationZones[] = kmlData.kml.Document.Folder.Placemark.map(placemark => {
+    let coordinates: [number, number][] = [];
+    const parseCoordinates = (coordinateString: string): [number, number][] => {


@stevekuznetsov I added a few unit tests for this. It looks like a recent package change may have caused a version conflict for "strip-ansi", causing the Jest tests to fail with "TypeError: stripAnsi is not a function". It looks like a dependency of "xml-js" may have changed the "strip-ansi" version.

I'm curious about the trade offs of breaking out smaller functions like parseCoordinates for testing. On the one hand this exposes them for individual unit test, but on the other they no longer have access to objects in the parent function (like logger).

stevekuznetsov · 2025-04-10T00:50:57Z

components/observations/ObservationsFilterForm.tsx

                    </View>
                  )}
                  {mapLayer && (
+                    // TODO: Render in two groups with respective labels: <Center_ID> Forecast Zones & Other Regions


@rustynwac let's sync on this

stevekuznetsov · 2025-04-10T00:53:14Z

components/observations/ObservationsListView.tsx

  const [filterConfig, setFilterConfig] = useState<ObservationFilterConfig>(originalFilterConfig);
  const [filterModalVisible, {set: setFilterModalVisible, on: showFilterModal, off: hideFilterModal}] = useToggle(false);
+  const avalancheZoneMetadata = useAvalancheCenterMetadata(center_id);
+  const alternateZonesUrl: string = (avalancheZoneMetadata.data?.widget_config?.observation_viewer?.alternate_zones as string) || '';


alternate_zones should be string | null today - no need for as string assertion, since we need to handle null (or, you coerce with || '' to empty string)

Oh, good point. Thanks!

stevekuznetsov · 2025-04-10T00:55:03Z

components/observations/ObservationsListView.tsx

  const originalFilterConfig: ObservationFilterConfig = useMemo(() => createDefaultFilterConfig(additionalFilters), [additionalFilters]);
  const [filterConfig, setFilterConfig] = useState<ObservationFilterConfig>(originalFilterConfig);
  const [filterModalVisible, {set: setFilterModalVisible, on: showFilterModal, off: hideFilterModal}] = useToggle(false);
+  const avalancheZoneMetadata = useAvalancheCenterMetadata(center_id);


super-duper nit: we prefer to suffix with Result for the tanstack-query result object (applies here and to the useAlternateObservationZones return)

Oh, that is a smart convention. Added it!

stevekuznetsov · 2025-04-10T00:55:26Z

components/observations/ObservationsListView.tsx

  const [filterModalVisible, {set: setFilterModalVisible, on: showFilterModal, off: hideFilterModal}] = useToggle(false);
+  const avalancheZoneMetadata = useAvalancheCenterMetadata(center_id);
+  const alternateZonesUrl: string = (avalancheZoneMetadata.data?.widget_config?.observation_viewer?.alternate_zones as string) || '';
+  const {data: alternateObservationZones} = useAlternateObservationZones(alternateZonesUrl);


Don't spread out data, we use the full result for e.g. the NotFound page

stevekuznetsov · 2025-04-10T00:56:58Z

components/observations/ObservationsListView.tsx


+  const mergedMapLayer = useMemo(() => {
+    if (alternateObservationZones && mapLayer) {
+      const zonesNotInMapLayer: AlternateObservationZones = alternateObservationZones.filter(


TypeScript compiler seems unhappy here - are we correctly typing the alternateObservationZones as a FeatureCollection only?

Gosh, I went around and around on this; in the end, the only way I was able to prevent the mergedMapLayer from throwing a bunch of type errors, was to create a new type MergedMapLayer, that supports a union z.union([mapLayerFeatureSchema, observationZonesFeatureSchema]);. Totally open to other solutions if I'm missing something!

stevekuznetsov · 2025-04-10T00:59:31Z

hooks/useAlternateObservationZones.ts

+};
+
+function queryKey(url: string) {
+  return ['alternateZoneKML-', url];


Suggested change

return ['alternateZoneKML-', url];

return ['alternateZoneKML', {url: url}];

hooks/useAlternateObservationZones.test.tsx

stevekuznetsov · 2025-04-10T01:02:01Z

hooks/useAlternateObservationZones.ts

+
+export function parseKmlData(response: string): AlternateObservationZones {
+  const kmlDataJson = xml2json(response, {compact: true, spaces: 2});
+  const kmlData = JSON.parse(kmlDataJson) as KMLData;


What we're doing in the rest of the codebase is using Zod to make sure that JSON.parse() spits out an object with the fields we're actually expecting. I think you already have a schema for KMLData, so we can zod.safeParse() the input and catch errors here instead of failing at runtime when we incorrectly access a field that we expected to exist but is missing.

This is a really cool capability, thanks for pointing me towards it; I added it here and in my transform function. One approach at getting the feature collections to merge involved populating the feature properties with default values to make them compatible with the mapLayer. For this the safeParse was super helpful!

stevekuznetsov · 2025-04-10T01:02:56Z

hooks/useAlternateObservationZones.ts

+    const feature: AlternateObservationZones = {
+      type: 'Feature',
+      geometry: {
+        type: Array.isArray(coordinates[0][0]) ? 'MultiPolygon' : 'Polygon',


I might be looking at an older version of this - did you decide if it would work to collapse this to MultiPolygon in every case?

stevekuznetsov · 2025-04-10T01:03:27Z

types/nationalAvalancheCenter/schemas.ts

+});
+export type KMLData = z.infer<typeof KMLDataSchema>;
+
+const AlternateObservationZonesSchema = z.union([


Ooops, sorry - finished the review before realizing you had not yet updated the code 🤦

Yeah, shoot - I should have let you know I was still grappling with the type issues. I've picked up the comments above, and committed all of my changes!

stevekuznetsov · 2025-04-14T13:47:57Z

components/observations/ObservationsListView.tsx

+
+  const mergedMapLayer = useMemo((): MergedMapLayer => {
+    if (!mapLayer || !observationOnlyZones || !observationOnlyZones.features || observationOnlyZones.features.length === 0) {
+      return mapLayer as MergedMapLayer;


as is 99.999999% of the time a signal that something is wrong - either the types as written or some logic around them. Since TypeScript uses structural typing, as long as the shape of mapLayer matches MergedMapLayer this should pass without any assertion necessary

(so, we should try to never use as and see what needs to be done to remove the uses that we have)

Thanks - I'll file that away as something to look out for in the future!

stevekuznetsov · 2025-04-14T13:57:56Z

hooks/useAlternateObservationZones.ts

+    queryKey: key,
+    queryFn: (): Promise<ObservationZonesFeatureCollection> => fetchAlternateObservationZones(thisLogger, url, center_id),
+    enabled: !!url,
+    cacheTime: 60, // TODO: Change cache to one day after testing


Couple TODOs left over?

stevekuznetsov · 2025-04-14T13:59:51Z

hooks/useAlternateObservationZones.ts

+  const kmlFile = KMLFileSchema.safeParse(JSON.parse(kmlResponse));
+
+  if (!kmlFile.success) {
+    logger.error(`Invalid KML file: ${JSON.stringify(kmlFile.error.format())}`);


Love it - so if this is not successful, you've got the code falling back to the zones from the NAC and failing gracefully. Great!

Can we also use Sentry.captureException to capture this (see other hooks for examples) to help us get notified if something is parsing wrong?

stevekuznetsov · 2025-04-14T14:00:42Z

hooks/useAlternateObservationZones.ts

+        type: 'Polygon',
+        coordinates: [coordinates],
+      },
+      id: Math.floor(Math.random() * 9000000) + 1000000, //Todo: replace with a better id generation method


What's the requirement for these IDs? Could we just use the iteration index from placemarks.map?

stevekuznetsov

Looking awesome! Just a couple small comments and I think we're good to go!

stevekuznetsov · 2025-04-14T14:03:26Z

There's something goofy going on with your package.json changes - did you intend to touch strip-ansi et al? If not, I would recommend resetting your changes to package.json and yarn.lock and running the minimal set of yarn add commands you need to pull in the KML bits, I think something you changed in yarn.lock (maybe unintentionally) is not acting nice with the CI.

yuliadub · 2025-08-11T19:25:17Z

added the custom zone name to the individual obs view (for some reason maps are not loading for me, ignore that :D )

before

now

hooks/useAvalancheCenterMetadata.ts

hooks/useAlternateObservationZones.ts

wheninseattle added 4 commits February 14, 2025 00:14

data: fetch and parse alternate zones

434e5b1

Merge alternateZones with mapLayer - outline next steps

b153c44

Add types

704d2d6

Decouple mapLayer context and alt zones. Filter zones with no observa…

5381a3e

…tions

stevekuznetsov reviewed Feb 26, 2025

View reviewed changes

components/observations/ObservationsFilterForm.tsx Outdated Show resolved Hide resolved

stevekuznetsov reviewed Feb 26, 2025

View reviewed changes

components/observations/ObservationsListView.tsx Outdated Show resolved Hide resolved

stevekuznetsov reviewed Feb 26, 2025

View reviewed changes

components/observations/ObservationsListView.tsx Outdated Show resolved Hide resolved

stevekuznetsov reviewed Feb 26, 2025

View reviewed changes

components/observations/ObservationsListView.tsx Outdated Show resolved Hide resolved

stevekuznetsov reviewed Feb 26, 2025

View reviewed changes

hooks/useAlternateObservationZones.ts Outdated Show resolved Hide resolved

stevekuznetsov reviewed Feb 26, 2025

View reviewed changes

hooks/useAlternateObservationZones.ts Outdated Show resolved Hide resolved

stevekuznetsov reviewed Feb 26, 2025

View reviewed changes

hooks/useAlternateObservationZones.ts Outdated Show resolved Hide resolved

stevekuznetsov reviewed Feb 26, 2025

View reviewed changes

wheninseattle added 7 commits March 3, 2025 22:41

Add hooks to incomplete query block and remove mapLayer feature filter

0aac541

WIP

4a58064

Merge branch 'main' into wn-889-add-observation-zones

0daa506

Improve types

0d7f3f2

Add prefetching

4021e1a

Break out functions and add unit tests

21f132e

Change KML typing to Zod

f276c99

wheninseattle commented Apr 4, 2025

View reviewed changes

Merge branch 'main' into wn-889-add-observation-zones

94b5e6e

stevekuznetsov reviewed Apr 10, 2025

View reviewed changes

wheninseattle added 2 commits April 10, 2025 17:54

Improve type safety and break out functions

cfa8ebd

Add tests for KML parsing

726d1ba

Pick up code review comments

6ac7afb

stevekuznetsov reviewed Apr 14, 2025

View reviewed changes

wheninseattle added 3 commits April 16, 2025 23:40

Implement prefetching

84358de

Address comments

8e15125

Fix package issues

f1575de

wheninseattle force-pushed the wn-889-add-observation-zones branch from d2748a1 to f1575de Compare April 17, 2025 07:25

yuliadub added 4 commits July 3, 2025 22:56

making ci happy

8a4c0b2

get changes from avy main

23c644d

make prettier happy

67b712b

add custom zone name to obs detail view header

8033735

yuliadub and others added 5 commits August 19, 2025 21:14

adjust xml parsing package and fix yarn lock

02b7643

revert yarn.lock changes

0acef2c

remove random yarn files

450bd48

adjust yarn.lock and fix package.json

dabca18

fix tests

0dff730

yuliadub mentioned this pull request Aug 23, 2025

package.json: bump to Expo 53.0.20 #955

Merged

yuliadub reviewed Aug 24, 2025

View reviewed changes

hooks/useAvalancheCenterMetadata.ts Show resolved Hide resolved

yuliadub force-pushed the wn-889-add-observation-zones branch from da73980 to 6694fab Compare August 24, 2025 04:52

stevekuznetsov reviewed Sep 5, 2025

View reviewed changes

hooks/useAlternateObservationZones.ts Outdated Show resolved Hide resolved

stevekuznetsov reviewed Sep 5, 2025

View reviewed changes

hooks/useAlternateObservationZones.ts Show resolved Hide resolved

fix zod errors

ed223ca

yuliadub force-pushed the wn-889-add-observation-zones branch from 6694fab to ed223ca Compare September 5, 2025 21:52

yuliadub and others added 2 commits September 5, 2025 19:18

merge main changes in

85dbe13

add fast-xml-parser back

92cbfdd

	return ['alternateZoneKML-', url];
	return ['alternateZoneKML', {url: url}];

Issue 889: Add Observation Only Zones #904

Are you sure you want to change the base?

Issue 889: Add Observation Only Zones #904

Uh oh!

Conversation

wheninseattle commented Feb 26, 2025

Description

Details

Related Issue

Acceptance Criteria

Considerations

Updates

Before

After

Testing Steps / QA Criteria

Uh oh!

Uh oh!

stevekuznetsov Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

stevekuznetsov Feb 26, 2025 •

edited

Loading