Fix schema check permission for proposal; fix other minor todos

Author: jdolle

packages/services/api/src/modules/proposals/providers/schema-proposal-storage.ts

@@ -130,16 +130,16 @@ export class SchemaProposalStorage {
       );
 
       const row = await conn.maybeOne(psql`
-        INSERT INTO schema_proposal_reviews
-          ("schema_proposal_id", "stage_transition", "author", "service_name")
-        VALUES (
-          ${args.id}
-          , ${args.stage}
-          , ${args.author}
-          , ${args.serviceName}
-        )
-        RETURNING ${schemaProposalReviewFields}
-      `);
+          INSERT INTO schema_proposal_reviews
+            ("schema_proposal_id", "stage_transition", "author", "service_name")
+          VALUES (
+            ${args.id}
+            , ${args.stage}
+            , ${args.author}
+            , ${args.serviceName}
+          )
+          RETURNING ${schemaProposalReviewFields}
+        `);
       return SchemaProposalReviewModel.parse(row);
     });
 
@@ -395,7 +395,7 @@ export class SchemaProposalStorage {
   ) {
     const values = records.map(
       r => psql`(
-      ${generateChangeHash(r.change)},
+      ${generateChangeHash(r.change)}
       ,${JSON.stringify(r.change)}
       ,${r.proposalId}
       ,${r.service ?? null}

packages/services/api/src/modules/proposals/resolvers/SchemaProposalChangeDetails.ts

@@ -17,7 +17,7 @@ export const SchemaProposalChangeDetails: SchemaProposalChangeDetailsResolvers =
       .getProposal({ id: schemaProposalId });
 
     if (!proposal) {
-      throw new Error('Something went wrong!');
+      throw new Error('Proposal not found');
     }
     const ref = await injector
       .get(IdTranslator)

packages/services/api/src/modules/schema/providers/registry-checks.ts

@@ -2,7 +2,6 @@ import { URL } from 'node:url';
 import { type GraphQLSchema } from 'graphql';
 import { Injectable, Scope } from 'graphql-modules';
 import hashObject from 'object-hash';
-import { generateChangeHash } from '@graphql-inspector/compare-changes';
 import { ChangeType, CriticalityLevel, DiffRule, TypeOfChangeType } from '@graphql-inspector/core';
 import type { CheckPolicyResponse } from '@hive/policy';
 import type { CompositionFailureError, ContractsInputType } from '@hive/schema';
@@ -737,18 +736,6 @@ export class RegistryChecks {
       );
     }
 
-    const test = await Promise.all(
-      inspectorChanges.map(async change => {
-        const hash = generateChangeHash(
-          change as unknown as Parameters<typeof generateChangeHash>[0],
-        );
-        // @todo batch lookup based on hashes
-        return hash;
-      }),
-    );
-    // @todo
-    console.log(JSON.stringify(test));
-
     let isFailure = false;
     const safeChanges: Array<SchemaChangeType> = [];
     const breakingChanges: Array<SchemaChangeType> = [];

packages/services/api/src/modules/schema/providers/schema-publisher.ts

@@ -687,10 +687,6 @@ export class SchemaPublisher {
           }
         }
 
-        // The check is where the changes are determined... Should this also be where
-        // it's associated proposal is found?...
-        // While I don't love this, it does make returning the change record nicer.
-        // @todo
         checkResult = await this.models[project.type].check({
           input: {
             sdl,

packages/services/api/src/modules/schema/resolvers/Mutation/schemaCheck.ts

@@ -1,16 +1,43 @@
+import { HiveError } from '../../../../shared/errors';
+import { SchemaProposalManager } from '../../../proposals/providers/schema-proposal-manager';
+import { IdTranslator } from '../../../shared/providers/id-translator';
 import { SchemaPublisher } from '../../providers/schema-publisher';
 import type { MutationResolvers } from './../../../../__generated__/types';
 
 export const schemaCheck: NonNullable<MutationResolvers['schemaCheck']> = async (
   _,
   { input },
-  { injector },
+  { injector, session },
 ) => {
+  if (typeof input.schemaProposalId === 'string') {
+    const selector = await injector
+      .get(IdTranslator)
+      .resolveTargetReference({ reference: input.target ?? null });
+
+    if (selector?.targetId) {
+      await session.assertPerformAction({
+        action: 'schemaProposal:modify',
+        organizationId: selector.organizationId,
+        params: {
+          organizationId: selector.organizationId,
+          projectId: selector.projectId,
+          targetId: selector.targetId,
+        },
+      });
+      const proposal = await injector
+        .get(SchemaProposalManager)
+        .getProposal({ id: input.schemaProposalId });
+      if (proposal?.targetId !== selector?.targetId) {
+        throw new HiveError('Proposal not found');
+      }
+    }
+  }
+
   const result = await injector.get(SchemaPublisher).check({
     ...input,
     service: input.service?.toLowerCase(),
     target: input.target ?? null,
-    schemaProposalId: input.schemaProposalId, // @todo check permission
+    schemaProposalId: input.schemaProposalId,
   });
 
   if ('changes' in result) {