From a9d2e341eced2d062b39ed832b2ecdbd7526aec0 Mon Sep 17 00:00:00 2001
From: Chris Campbell <ccampbell@climateinteractive.org>
Date: Mon, 25 Mar 2024 13:00:25 -0700
Subject: [PATCH] fix: correct handling of the case where comparison scenario
 is not valid for one side (#451)

Fixes #450
---
 examples/sample-check-bundle/src/graphs.ts    |  9 +++++++
 .../src/comparisons/comparisons.yaml          | 19 +++++++++++++++
 .../src/components/graphs/context-graph-vm.ts | 16 +++++++++++--
 .../components/graphs/context-graph.svelte    | 24 ++++++++++++-------
 4 files changed, 57 insertions(+), 11 deletions(-)

diff --git a/examples/sample-check-bundle/src/graphs.ts b/examples/sample-check-bundle/src/graphs.ts
index 33ed0dcc..efa3d908 100644
--- a/examples/sample-check-bundle/src/graphs.ts
+++ b/examples/sample-check-bundle/src/graphs.ts
@@ -148,6 +148,15 @@ export function getGraphSpecs(modelVersion: number, outputVars: Map<DatasetKey,
 
   const graphSpecs: BundleGraphSpec[] = []
   for (let i = 1; i <= 8; i++) {
+    if (i === 1 && modelVersion === 1) {
+      // Simulate the case where a graph is not included in version 1 but is added
+      // in version 2
+      continue
+    } else if (i === 2 && modelVersion === 2) {
+      // Simulate the case where a graph is included in version 1 but is removed
+      // in version 2
+      continue
+    }
     graphSpecs.push(graphSpec(`${i}`, `Sample Graph ${i}`))
   }
 
diff --git a/examples/sample-check-tests/src/comparisons/comparisons.yaml b/examples/sample-check-tests/src/comparisons/comparisons.yaml
index 2e12339e..232a0e31 100644
--- a/examples/sample-check-tests/src/comparisons/comparisons.yaml
+++ b/examples/sample-check-tests/src/comparisons/comparisons.yaml
@@ -73,6 +73,7 @@
 #
 # Variable does not exist in left or right (unknown input)
 - scenario:
+    id: input_q_at_90
     title: Input Q (unknown)
     subtitle: at 90
     with: Input Q
@@ -80,10 +81,28 @@
 
 # Variable exists in left+right, but value is invalid in left+right
 - scenario:
+    id: input_a_at_666
     title: Input A
     subtitle: at 666
     with: Input A
     at: 666
+
+# Scenario group that includes error scenarios from above
+- scenario_group:
+    id: G1
+    title: Scenario Group 3
+    scenarios:
+      - scenario_ref: input_q_at_90
+      - scenario_ref: input_a_at_666
+
+# View group refers to scenario that has unknown inputs
+- view_group:
+    title: Group 3 (errors are expected)
+    scenarios:
+      - scenario_group_ref: G1
+    graphs:
+      - '3'
+      - '4'
 # TODO: Input ID exists in both left and right, but with different variable names
 
 # TODO: Input ID exists in both left and right, but with different min/max
diff --git a/packages/check-ui-shell/src/components/graphs/context-graph-vm.ts b/packages/check-ui-shell/src/components/graphs/context-graph-vm.ts
index fec3f7bc..2ea8d594 100644
--- a/packages/check-ui-shell/src/components/graphs/context-graph-vm.ts
+++ b/packages/check-ui-shell/src/components/graphs/context-graph-vm.ts
@@ -41,14 +41,26 @@ export class ContextGraphViewModel {
     this.datasetClass = `dataset-color-${bundle === 'right' ? '1' : '0'}`
     if (graphSpec) {
       const scenarioSpec = bundle === 'right' ? scenario.specR : scenario.specL
-      this.linkItems = b.model.getGraphLinksForScenario(scenarioSpec, graphSpec.id)
-      this.requestKey = `context-graph::${requestId++}::${bundle}::${graphSpec.id}::${scenario.key}`
+      if (scenarioSpec !== undefined) {
+        this.linkItems = b.model.getGraphLinksForScenario(scenarioSpec, graphSpec.id)
+        this.requestKey = `context-graph::${requestId++}::${bundle}::${graphSpec.id}::${scenario.key}`
+      } else {
+        this.linkItems = []
+      }
       this.writableContent = writable(undefined)
       this.content = this.writableContent
     }
   }
 
   requestData(): void {
+    if (this.requestKey === undefined) {
+      // The request key is undefined when the scenario is invalid or unavailable for the
+      // requested side; in this case, we can set the flags and return early
+      this.dataRequested = true
+      this.dataLoaded = true
+      return
+    }
+
     if (this.dataRequested) {
       return
     }
diff --git a/packages/check-ui-shell/src/components/graphs/context-graph.svelte b/packages/check-ui-shell/src/components/graphs/context-graph.svelte
index ce3fd07a..240c6eef 100644
--- a/packages/check-ui-shell/src/components/graphs/context-graph.svelte
+++ b/packages/check-ui-shell/src/components/graphs/context-graph.svelte
@@ -69,15 +69,20 @@ include context-graph.pug
   +if('viewModel.graphSpec')
     .graph-and-info
       .graph-title(class!='{viewModel.datasetClass}') { @html viewModel.graphSpec.title }
-      .graph-container
-        Lazy(bind:visible!='{visible}')
-          +if('$content')
-            Graph(viewModel!='{$content.graphData}' config!='{graphConfig}')
-      Legend(graphSpec!='{viewModel.graphSpec}')
-      +links
+      +if('viewModel.requestKey')
+        .graph-container
+          Lazy(bind:visible!='{visible}')
+            +if('$content && $content.graphData')
+              Graph(viewModel!='{$content.graphData}' config!='{graphConfig}')
+        Legend(graphSpec!='{viewModel.graphSpec}')
+        +links
+        +else
+          .message.not-shown
+            span Graph not shown: scenario is invalid in&nbsp;
+            span(class!='{viewModel.datasetClass}') { viewModel.bundleName }
     +else
-      .message
-        span Not included in&nbsp;
+      .message.not-included
+        span Graph not included in&nbsp;
         span(class!='{viewModel.datasetClass}') { viewModel.bundleName }
 
 </template>
@@ -122,8 +127,9 @@ $graph-height: 20rem
   min-height: $graph-height
   align-items: center
   justify-content: center
-  background-color: #555
   color: #aaa
+  &.not-included
+    background-color: #555
 
 .link-container
   display: flex