fix(server): viewerResources failing w/ nonexistent view (#5271)

packages/server/modules/viewer/graph/resolvers/viewerResources.ts

@@ -40,7 +40,8 @@ const extendedViewerResourcesResolver = async (
     const canRead = await ctx.authPolicies.project.savedViews.canRead({
       userId: ctx.userId,
       projectId,
-      savedViewId
+      savedViewId,
+      allowNonExistent: true // ignore missing view
     })
     throwIfAuthNotOk(canRead)
   }

packages/server/modules/viewer/services/viewerResources.ts

@@ -14,7 +14,6 @@ import type {
   ViewerResourceItem
 } from '@/modules/core/graph/generated/graphql'
 import type { CommitRecord } from '@/modules/core/helpers/types'
-import { NotFoundError } from '@/modules/shared/errors'
 import { getFeatureFlags } from '@/modules/shared/helpers/envHelper'
 import type { DependenciesOf } from '@/modules/shared/helpers/factory'
 import type {
@@ -370,9 +369,9 @@ const adjustResourceIdStringWithSpecificSavedViewSettingsFactory =
       : params.savedViewId
 
     if (!savedView) {
-      throw new NotFoundError(
-        `Saved view with ID ${params.savedViewId} not found in project ${projectId}`
-      )
+      // We don't want this to fail, cause this would break the app when nonexistant views are referenced
+      // Just ignore the view
+      return emptyReturn
     }
 
     const savedViewResources = resourceBuilder().addFromString(savedView.resourceIds)

packages/server/modules/viewer/tests/integration/viewerResources.spec.ts

@@ -10,7 +10,6 @@ import {
 } from '@/modules/core/repositories/commits'
 import { getStreamObjectsFactory } from '@/modules/core/repositories/objects'
 import { buildBasicTestProject } from '@/modules/core/tests/helpers/creation'
-import { NotFoundError } from '@/modules/shared/errors'
 import { getFeatureFlags } from '@/modules/shared/helpers/envHelper'
 import type { ViewerResourceGroup } from '@/modules/viewer/domain/types/resources'
 import type { SavedView } from '@/modules/viewer/domain/types/savedViews'
@@ -25,7 +24,7 @@ import {
   viewerResourcesToString
 } from '@/modules/viewer/services/viewerResources'
 import { createTestSavedView } from '@/modules/viewer/tests/helpers/savedViews'
-import { expectToThrow, itEach } from '@/test/assertionHelper'
+import { itEach } from '@/test/assertionHelper'
 import type { BasicTestUser } from '@/test/authHelper'
 import { buildBasicTestUser, createTestUser } from '@/test/authHelper'
 import {
@@ -323,21 +322,20 @@ describe('Viewer Resources Collection Service', () => {
           secondModelBasicView = views[1]
         })
 
-        it('throws if setting nonexistant view', async () => {
+        it('should not throw if setting nonexistant view', async () => {
           const sut = buildSUT()
 
-          const err = await expectToThrow(
-            async () =>
-              await sut({
-                projectId: myProject.id,
-                resourceIdString: resourceBuilder()
-                  .addResources(firstModel().id)
-                  .toString(),
-                savedViewId: 'aaa'
-              })
-          )
-          expect(err instanceof NotFoundError).to.be.true
-          expect(err.message).to.include('Saved view')
+          // shouldnt throw so that deleting a view while its referred to in the URL doesn't
+          // cause the entire app to break - just ignore the value then
+          const res = await sut({
+            projectId: myProject.id,
+            resourceIdString: resourceBuilder()
+              .addResources(firstModel().id)
+              .toString(),
+            savedViewId: 'aaa'
+          })
+
+          expect(res).to.be.ok
         })
 
         itEach(

packages/shared/src/authz/fragments/savedViews.spec.ts

@@ -229,6 +229,24 @@ describe('ensureCanAccessSavedViewFragment', () => {
         })
       }
     )
+
+    it.each(<const>['read', 'write'])(
+      `doesn't fail if view doesnt exist and allowNonExistent set (%s)`,
+      async (access) => {
+        const sut = buildWorkspaceSUT({
+          getSavedView: async () => null
+        })
+
+        const result = await sut({
+          userId,
+          projectId,
+          savedViewId,
+          access,
+          allowNonExistent: true
+        })
+        expect(result).toBeAuthOKResult()
+      }
+    )
   })
 })
 

packages/shared/src/authz/fragments/savedViews.ts

@@ -52,6 +52,10 @@ export const ensureCanAccessSavedViewFragment: AuthPolicyEnsureFragment<
     ProjectContext &
     SavedViewContext & {
       access: 'read' | 'write'
+      /**
+       * In some cases we want to just ignore a view being non-existant, instead of throwing
+       */
+      allowNonExistent?: boolean
     },
   InstanceType<
     | typeof SavedViewNotFoundError
@@ -71,7 +75,7 @@ export const ensureCanAccessSavedViewFragment: AuthPolicyEnsureFragment<
   >
 > =
   (loaders) =>
-  async ({ userId, projectId, savedViewId, access }) => {
+  async ({ userId, projectId, savedViewId, access, allowNonExistent }) => {
     const canUseSavedViews = await ensureCanUseProjectWorkspacePlanFeatureFragment(
       loaders
     )({
@@ -81,7 +85,10 @@ export const ensureCanAccessSavedViewFragment: AuthPolicyEnsureFragment<
     if (canUseSavedViews.isErr) return err(canUseSavedViews.error)
 
     const savedView = await loaders.getSavedView({ projectId, savedViewId })
-    if (!savedView) return err(new SavedViewNotFoundError())
+    if (!savedView) {
+      if (allowNonExistent) return ok()
+      return err(new SavedViewNotFoundError())
+    }
 
     const isPublic = savedView.visibility === SavedViewVisibility.public
     if (isPublic && access === 'read') {

packages/shared/src/authz/policies/project/savedViews/canRead.ts

@@ -34,7 +34,14 @@ export const canReadSavedViewPolicy: AuthPolicy<
   | typeof Loaders.getWorkspaceSsoProvider
   | typeof Loaders.getWorkspaceSsoSession
   | typeof Loaders.getProjectRole,
-  MaybeUserContext & ProjectContext & SavedViewContext,
+  MaybeUserContext &
+    ProjectContext &
+    SavedViewContext & {
+      /**
+       * In some cases we want to just ignore a view being non-existant, instead of throwing
+       */
+      allowNonExistent?: boolean
+    },
   InstanceType<
     | typeof SavedViewNotFoundError
     | typeof SavedViewNoAccessError
@@ -53,11 +60,12 @@ export const canReadSavedViewPolicy: AuthPolicy<
   >
 > =
   (loaders) =>
-  async ({ userId, projectId, savedViewId }) => {
+  async ({ userId, projectId, savedViewId, allowNonExistent }) => {
     return await ensureCanAccessSavedViewFragment(loaders)({
       userId,
       projectId,
       savedViewId,
-      access: 'read'
+      access: 'read',
+      allowNonExistent
     })
   }