Plan store refactor (#1179)

* Refactor model page and models subscription into a new store
* Reset model store on models page unmount
* Regression test

e2e-tests/fixtures/Models.ts

@@ -22,9 +22,13 @@ export class Models {
     this.updatePage(page);
   }
 
-  async createModel() {
+  async createModel(modelName = '') {
     await expect(this.tableRow).not.toBeVisible();
-    await this.fillInputName();
+    if (modelName) {
+      await this.fillInputName(modelName);
+    } else {
+      await this.fillInputName();
+    }
     await this.fillInputVersion();
     await this.fillInputFile();
     await this.createButton.click();

e2e-tests/tests/models.test.ts

@@ -1,13 +1,16 @@
 import test, { expect, type BrowserContext, type Page } from '@playwright/test';
+import { Constraints } from '../fixtures/Constraints.js';
 import { Models } from '../fixtures/Models.js';
 let context: BrowserContext;
 let models: Models;
+let constraints: Constraints;
 let page: Page;
 
 test.beforeAll(async ({ browser }) => {
   context = await browser.newContext();
   page = await context.newPage();
   models = new Models(page);
+  constraints = new Constraints(page, models);
   await models.goto();
 });
 
@@ -71,5 +74,32 @@ test.describe.serial('Models', () => {
     await expect(models.inputFile).toBeEmpty();
     await expect(models.inputVersion).toBeEmpty();
     await expect(models.inputName).toBeEmpty();
+
+    // Cleanup
+    await models.deleteModel();
+  });
+
+  test('Model creation errors should clear on page destroy', async () => {
+    // Create model
+    await models.createModel(models.modelName);
+
+    // Create model again with the same name
+    await models.fillInputName(models.modelName);
+    await models.fillInputVersion();
+    await models.fillInputFile();
+    await models.createButton.click();
+
+    // Expect an error to be present
+    await expect(models.alertError).toBeVisible();
+
+    // Navigate away and back
+    await constraints.goto();
+    await models.goto();
+
+    // Expect no error
+    await expect(models.alertError).not.toBeVisible();
+
+    // Cleanup
+    await models.deleteModel();
   });
 });

src/components/expansion/ExpansionRuleForm.svelte

@@ -5,7 +5,8 @@
   import { base } from '$app/paths';
   import { onMount } from 'svelte';
   import { createExpansionRuleError, expansionRulesFormColumns, savingExpansionRule } from '../../stores/expansion';
-  import { activityTypes, models } from '../../stores/plan';
+  import { models } from '../../stores/model';
+  import { activityTypes } from '../../stores/plan';
   import { commandDictionaries } from '../../stores/sequencing';
   import { tags } from '../../stores/tags';
   import type { User, UserId } from '../../types/app';

src/components/expansion/ExpansionSetForm.svelte

@@ -5,7 +5,7 @@
   import { base } from '$app/paths';
   import type { ICellRendererParams, ValueGetterParams } from 'ag-grid-community';
   import { expansionSetsFormColumns, savingExpansionSet } from '../../stores/expansion';
-  import { models } from '../../stores/plan';
+  import { models } from '../../stores/model';
   import { commandDictionaries } from '../../stores/sequencing';
   import type { ActivityTypeExpansionRules } from '../../types/activity';
   import type { User } from '../../types/app';

src/components/ui/Association/DefinitionEditor.svelte

@@ -3,7 +3,7 @@
 <script lang="ts">
   import type { editor as Editor, languages } from 'monaco-editor/esm/vs/editor/editor.api';
   import { createEventDispatcher } from 'svelte';
-  import { models } from '../../../stores/plan';
+  import { models } from '../../../stores/model';
   import type { DropdownOptions, SelectedDropdownOptionValue } from '../../../types/dropdown';
   import type { Monaco, TypeScriptFile } from '../../../types/monaco';
   import MonacoEditor from '../../ui/MonacoEditor.svelte';

src/routes/models/+page.svelte

@@ -5,7 +5,7 @@
   import { base } from '$app/paths';
   import type { ICellRendererParams, ValueGetterParams } from 'ag-grid-community';
   import BarChartIcon from 'bootstrap-icons/icons/bar-chart.svg?component';
-  import { onMount } from 'svelte';
+  import { onDestroy, onMount } from 'svelte';
   import Nav from '../../components/app/Nav.svelte';
   import PageTitle from '../../components/app/PageTitle.svelte';
   import AlertError from '../../components/ui/AlertError.svelte';
@@ -14,7 +14,7 @@
   import SingleActionDataGrid from '../../components/ui/DataGrid/SingleActionDataGrid.svelte';
   import Panel from '../../components/ui/Panel.svelte';
   import SectionTitle from '../../components/ui/SectionTitle.svelte';
-  import { createModelError, creatingModel, models } from '../../stores/plan';
+  import { createModelError, creatingModel, models, resetModelStores } from '../../stores/model';
   import type { User } from '../../types/app';
   import type { DataGridColumnDef, RowId } from '../../types/data-grid';
   import type { ModelSlim } from '../../types/model';
@@ -62,7 +62,7 @@
 
   let columnDefs: DataGridColumnDef[] = baseColumnDefs;
   let createButtonDisabled: boolean = false;
-  let files: FileList;
+  let files: FileList | undefined;
   let hasCreatePermission: boolean = false;
   let hasDeletePermission: boolean = false;
   let name = '';
@@ -115,6 +115,10 @@
     models.updateValue(() => data.initialModels);
   });
 
+  onDestroy(() => {
+    resetModelStores();
+  });
+
   function deleteModel(model: ModelSlim) {
     effects.deleteModel(model, user);
   }
@@ -132,9 +136,12 @@
   }
 
   async function submitForm(e: SubmitEvent) {
-    await effects.createModel(name, version, files, data.user, description);
-    if ($createModelError === null && e.target instanceof HTMLFormElement) {
-      e.target.reset();
+    if (files) {
+      await effects.createModel(name, version, files, data.user, description);
+      if ($createModelError === null && e.target instanceof HTMLFormElement) {
+        e.target.reset();
+        files = undefined; // reset list of files since they are not reset on form reset
+      }
     }
   }
 </script>

src/stores/model.ts

@@ -0,0 +1,20 @@
+import { writable, type Writable } from 'svelte/store';
+import type { ModelSlim } from '../types/model';
+import gql from '../utilities/gql';
+import { gqlSubscribable } from './subscribable';
+
+/* Writeable. */
+export const creatingModel: Writable<boolean> = writable(false);
+
+export const createModelError: Writable<string | null> = writable(null);
+
+/* Subscriptions. */
+
+export const models = gqlSubscribable<ModelSlim[]>(gql.SUB_MODELS, {}, [], null);
+
+/* Helper Functions. */
+
+export function resetModelStores() {
+  creatingModel.set(false);
+  createModelError.set(null);
+}

src/stores/plan.ts

@@ -1,6 +1,5 @@
 import { derived, writable, type Readable, type Writable } from 'svelte/store';
 import type { ActivityType } from '../types/activity';
-import type { ModelSlim } from '../types/model';
 import type { Plan, PlanMergeRequest, PlanMergeRequestSchema } from '../types/plan';
 import type { PlanDataset } from '../types/simulation';
 import type { Tag } from '../types/tags';
@@ -22,10 +21,6 @@ export const planReadOnly: Readable<boolean> = derived(
   ([$planReadOnlySnapshot, $planReadOnlyMergeRequest]) => $planReadOnlyMergeRequest || $planReadOnlySnapshot,
 );
 
-export const creatingModel: Writable<boolean> = writable(false);
-
-export const createModelError: Writable<string | null> = writable(null);
-
 export const createPlanError: Writable<string | null> = writable(null);
 
 export const creatingPlan: Writable<boolean> = writable(false);
@@ -54,8 +49,6 @@ export const planTags = gqlSubscribable<Tag[]>(gql.SUB_PLAN_TAGS, { planId }, []
   tags.map((tag: { tag: Tag }) => tag.tag),
 );
 
-export const models = gqlSubscribable<ModelSlim[]>(gql.SUB_MODELS, {}, [], null);
-
 export const planDatasets = gqlSubscribable<PlanDataset[] | null>(gql.SUB_PLAN_DATASET, { planId }, null, null);
 
 export const planLocked = gqlSubscribable<boolean>(
@@ -96,8 +89,6 @@ export const planRevision = gqlSubscribable<number>(
 
 export function resetPlanStores() {
   activityEditingLocked.set(false);
-  creatingModel.set(false);
-  createModelError.set(null);
   createPlanError.set(null);
   creatingPlan.set(false);
   plan.set(null);

src/utilities/effects.ts

@@ -15,7 +15,8 @@ import {
   savingExpansionRule,
   savingExpansionSet,
 } from '../stores/expansion';
-import { createModelError, createPlanError, creatingModel, creatingPlan, models, planId } from '../stores/plan';
+import { createModelError, creatingModel, models } from '../stores/model';
+import { createPlanError, creatingPlan, planId } from '../stores/plan';
 import { schedulingRequests, selectedSpecId } from '../stores/scheduling';
 import { commandDictionaries } from '../stores/sequencing';
 import { selectedSpanId, simulationDataset, simulationDatasetId } from '../stores/simulation';