Fix missing list of errors on the history page (#1586)

integration-tests/testkit/flow.ts

@@ -799,6 +799,12 @@ export function fetchLatestSchema(token: string) {
             }
             total
           }
+          errors {
+            nodes {
+              message
+            }
+            total
+          }
         }
       }
     `),

packages/services/api/src/modules/schema/module.graphql.ts

@@ -389,6 +389,7 @@ export default gql`
     Experimental: This field is not stable and may change in the future.
     """
     explorer(usage: SchemaExplorerUsageInput): SchemaExplorer!
+    errors: SchemaErrorConnection!
   }
 
   type SchemaVersionConnection {

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

@@ -482,6 +482,29 @@ export const resolvers: SchemaModule.Resolvers = {
         target: version.target,
       });
     },
+    async errors(version, _, { injector }) {
+      const schemaManager = injector.get(SchemaManager);
+      const schemaHelper = injector.get(SchemaHelper);
+      const [schemas, project] = await Promise.all([
+        schemaManager.getSchemasOfVersion({
+          version: version.id,
+          organization: version.organization,
+          project: version.project,
+          target: version.target,
+        }),
+        injector.get(ProjectManager).getProject({
+          organization: version.organization,
+          project: version.project,
+        }),
+      ]);
+
+      const orchestrator = schemaManager.matchOrchestrator(project.type);
+
+      return orchestrator.validate(
+        schemas.map(s => schemaHelper.createSchemaObject(s)),
+        project.externalComposition,
+      );
+    },
     async supergraph(version, _, { injector }) {
       const project = await injector.get(ProjectManager).getProject({
         organization: version.organization,

packages/web/app/.storybook/preview.js

@@ -1,5 +1,11 @@
 import '../public/styles.css';
 
+console.log('[dark]');
+if (!document.body.className.includes('dark')) {
+  console.log('[dark] added');
+  document.body.className += ' ' + 'dark';
+}
+
 /** @type { import('@storybook/react').Preview } */
 const preview = {
   parameters: {

packages/web/app/pages/[orgId]/[projectId]/[targetId]/history.tsx

@@ -1,11 +1,10 @@
 import { ReactElement, useCallback, useState } from 'react';
 import NextLink from 'next/link';
 import { clsx } from 'clsx';
-import reactStringReplace from 'react-string-replace';
 import { useQuery } from 'urql';
 import { authenticated } from '@/components/authenticated-container';
-import { Label } from '@/components/common';
 import { TargetLayout } from '@/components/layouts';
+import { VersionErrorsAndChanges } from '@/components/target/history/errors-and-changes';
 import {
   Badge,
   Button,
@@ -19,64 +18,11 @@ import {
 } from '@/components/v2';
 import { DiffIcon } from '@/components/v2/icon';
 import { graphql } from '@/gql';
-import {
-  CompareDocument,
-  CriticalityLevel,
-  SchemaChangeFieldsFragment,
-  VersionsDocument,
-} from '@/graphql';
+import { CompareDocument, VersionsDocument } from '@/graphql';
 import { useRouteSelector } from '@/lib/hooks/use-route-selector';
 import { withSessionProtection } from '@/lib/supertokens/guard';
 import { CrossCircledIcon, RowsIcon } from '@radix-ui/react-icons';
 
-function labelize(message: string) {
-  const findSingleQuotes = /'([^']+)'/gim;
-
-  return reactStringReplace(message, findSingleQuotes, (match, i) => (
-    <Label key={i}>{match}</Label>
-  ));
-}
-
-const titleMap: Record<CriticalityLevel, string> = {
-  Safe: 'Safe Changes',
-  Breaking: 'Breaking Changes',
-  Dangerous: 'Dangerous Changes',
-};
-
-const criticalityLevelMapping = {
-  [CriticalityLevel.Safe]: clsx('text-emerald-400'),
-  [CriticalityLevel.Dangerous]: clsx('text-yellow-400'),
-} as Record<CriticalityLevel, string>;
-
-function ChangesBlock({
-  changes,
-  criticality,
-}: {
-  changes: SchemaChangeFieldsFragment[];
-  criticality: CriticalityLevel;
-}): ReactElement | null {
-  const filteredChanges = changes.filter(c => c.criticality === criticality);
-
-  if (!filteredChanges.length) {
-    return null;
-  }
-
-  return (
-    <div>
-      <h2 className="mb-2 text-lg font-medium text-gray-900 dark:text-white">
-        {titleMap[criticality]}
-      </h2>
-      <ul className="list-inside list-disc pl-3 text-base leading-relaxed">
-        {filteredChanges.map((change, key) => (
-          <li key={key} className={clsx(criticalityLevelMapping[criticality] ?? 'text-red-400')}>
-            <span className="text-gray-600 dark:text-white">{labelize(change.message)}</span>
-          </li>
-        ))}
-      </ul>
-    </div>
-  );
-}
-
 function DiffView({
   view,
   versionId,
@@ -95,6 +41,7 @@ function DiffView({
     },
   });
   const comparison = compareQuery.data?.schemaCompareToPrevious;
+  const compositionErrors = compareQuery.data?.schemaVersion?.errors;
   const { error } = compareQuery;
 
   if (error) {
@@ -114,7 +61,7 @@ function DiffView({
     );
   }
 
-  if (!comparison) {
+  if (!comparison || !compositionErrors) {
     return null;
   }
 
@@ -141,13 +88,7 @@ function DiffView({
     return <DiffEditor before={before} after={after} />;
   }
 
-  return (
-    <div className="space-y-3 p-6">
-      <ChangesBlock changes={comparison.changes.nodes} criticality={CriticalityLevel.Breaking} />
-      <ChangesBlock changes={comparison.changes.nodes} criticality={CriticalityLevel.Dangerous} />
-      <ChangesBlock changes={comparison.changes.nodes} criticality={CriticalityLevel.Safe} />
-    </div>
-  );
+  return <VersionErrorsAndChanges changes={comparison.changes} errors={compositionErrors} />;
 }
 
 // URQL's Infinite scrolling pattern

packages/web/app/src/components/target/history/errors-and-changes.tsx

@@ -0,0 +1,132 @@
+import { ReactElement } from 'react';
+import { clsx } from 'clsx';
+import reactStringReplace from 'react-string-replace';
+import { Label } from '@/components/common';
+import { Accordion } from '@/components/v2';
+import { CriticalityLevel, SchemaChangeFieldsFragment } from '@/graphql';
+
+function labelize(message: string) {
+  // Turn " into '
+  // Replace '...' with <Label>...</Label>
+  return reactStringReplace(message.replace(/"/g, "'"), /'([^']+)'/gim, (match, i) => {
+    return <Label key={i}>{match}</Label>;
+  });
+}
+
+const titleMap: Record<CriticalityLevel, string> = {
+  Safe: 'Safe Changes',
+  Breaking: 'Breaking Changes',
+  Dangerous: 'Dangerous Changes',
+};
+
+const criticalityLevelMapping = {
+  [CriticalityLevel.Safe]: clsx('text-emerald-400'),
+  [CriticalityLevel.Dangerous]: clsx('text-yellow-400'),
+} as Record<CriticalityLevel, string>;
+
+function ChangesBlock({
+  changes,
+  criticality,
+}: {
+  changes: SchemaChangeFieldsFragment[];
+  criticality: CriticalityLevel;
+}): ReactElement | null {
+  const filteredChanges = changes.filter(c => c.criticality === criticality);
+
+  if (!filteredChanges.length) {
+    return null;
+  }
+
+  return (
+    <div>
+      <h2 className="mb-2 text-lg font-medium text-gray-900 dark:text-white">
+        {titleMap[criticality]}
+      </h2>
+      <ul className="list-inside list-disc pl-3 text-base leading-relaxed">
+        {filteredChanges.map((change, key) => (
+          <li key={key} className={clsx(criticalityLevelMapping[criticality] ?? 'text-red-400')}>
+            <span className="text-gray-600 dark:text-white">{labelize(change.message)}</span>
+          </li>
+        ))}
+      </ul>
+    </div>
+  );
+}
+
+export function VersionErrorsAndChanges(props: {
+  changes: {
+    nodes: SchemaChangeFieldsFragment[];
+    total: number;
+  };
+  errors: {
+    nodes: Array<{
+      message: string;
+    }>;
+    total: number;
+  };
+}) {
+  const generalErrors = props.errors.nodes.filter(err => err.message.startsWith('[') === false);
+  const groupedServiceErrors = new Map<string, string[]>();
+
+  props.errors.nodes.forEach(err => {
+    if (err.message.startsWith('[')) {
+      const [service, ...message] = err.message.split('] ');
+      const serviceName = service.replace('[', '');
+      const errorMessage = message.join('] ');
+
+      if (!groupedServiceErrors.has(serviceName)) {
+        groupedServiceErrors.set(serviceName, [errorMessage]);
+      }
+
+      groupedServiceErrors.get(serviceName)!.push(errorMessage);
+    }
+  });
+
+  const serviceErrorEntries = Array.from(groupedServiceErrors.entries());
+
+  return (
+    <Accordion type="multiple" defaultValue={props.changes.total > 0 ? 'changes' : 'errors'}>
+      {props.changes.total ? (
+        <Accordion.Item value="changes">
+          <Accordion.Header>Changes</Accordion.Header>
+          <Accordion.Content>
+            <div className="space-y-3 p-6">
+              <ChangesBlock changes={props.changes.nodes} criticality={CriticalityLevel.Breaking} />
+              <ChangesBlock
+                changes={props.changes.nodes}
+                criticality={CriticalityLevel.Dangerous}
+              />
+              <ChangesBlock changes={props.changes.nodes} criticality={CriticalityLevel.Safe} />
+            </div>
+          </Accordion.Content>
+        </Accordion.Item>
+      ) : null}
+      {props.errors.total ? (
+        <Accordion.Item value="errors">
+          <Accordion.Header>Composition errors</Accordion.Header>
+          <Accordion.Content>
+            <ul className="list-inside list-disc pl-3 text-base leading-relaxed">
+              {generalErrors.map((error, key) => (
+                <li key={key}>
+                  <span className="text-gray-600 dark:text-white">{labelize(error.message)}</span>
+                </li>
+              ))}
+              {serviceErrorEntries.map(([service, errors]) => (
+                <li key={service}>
+                  <span className="text-gray-600 dark:text-white">{service}</span>
+                  <ul className="list-inside list-disc pl-3 text-base leading-relaxed">
+                    {errors.map((error, key) => (
+                      <li key={key}>
+                        <span className="text-gray-600 dark:text-white">{labelize(error)}</span>
+                      </li>
+                    ))}
+                  </ul>
+                </li>
+              ))}
+            </ul>
+          </Accordion.Content>
+        </Accordion.Item>
+      ) : null}
+    </Accordion>
+  );
+}

packages/web/app/src/graphql/query.compare.graphql

@@ -6,4 +6,15 @@ query compare($organization: ID!, $project: ID!, $target: ID!, $version: ID!) {
     ...SchemaCompareResultFields
     ...SchemaCompareErrorFields
   }
+  schemaVersion(
+    selector: { organization: $organization, project: $project, target: $target, version: $version }
+  ) {
+    id
+    errors {
+      nodes {
+        message
+      }
+      total
+    }
+  }
 }

packages/web/app/src/stories/accordion.stories.tsx

@@ -12,7 +12,7 @@ type Story = StoryObj<typeof Accordion>;
 
 export const Default: Story = {
   render: () => (
-    <div className="bg-white p-5">
+    <div className="p-5">
       <Accordion>
         <Accordion.Item value="First">
           <Accordion.Header>First</Accordion.Header>
@@ -35,7 +35,7 @@ export const Dynamic: Story = {
   render: () => {
     const [list, setList] = useState(['Tab #1', 'Tab #2', 'Tab #3']);
     return (
-      <div className="bg-white p-5">
+      <div className="p-5">
         <Accordion>
           {list.map(tab => (
             <Accordion.Item key={tab} value={tab}>

packages/web/app/src/stories/history-page.stories.tsx

@@ -0,0 +1,97 @@
+import { VersionErrorsAndChanges } from '@/components/target/history/errors-and-changes';
+import { CriticalityLevel } from '@/graphql';
+import type { Meta, StoryObj } from '@storybook/react';
+
+const changes = [
+  {
+    message: 'Type "Foo" was removed',
+    criticality: CriticalityLevel.Breaking,
+  },
+  {
+    message: 'Input field "limit" was added to input object type "Filter"',
+    criticality: CriticalityLevel.Breaking,
+  },
+  {
+    message: 'Field "User.nickname" is no longer deprecated',
+    criticality: CriticalityLevel.Dangerous,
+  },
+  {
+    message: 'Field "type" was added to object type "User"',
+    criticality: CriticalityLevel.Safe,
+  },
+];
+
+const errors = [
+  {
+    message: 'Field "Foo.id" can only be defined once.',
+  },
+  {
+    message:
+      '[subgraph-a] Foo.name -> is marked as @external but is not used by a @requires, @key, or @provides directive.',
+  },
+  {
+    message:
+      '[subgraph-b] Foo.name -> is marked as @external but is not used by a @requires, @key, or @provides directive.',
+  },
+];
+
+const meta: Meta<typeof VersionErrorsAndChanges> = {
+  title: 'VersionErrorsAndChanges',
+  component: VersionErrorsAndChanges,
+};
+
+export default meta;
+type Story = StoryObj<typeof VersionErrorsAndChanges>;
+
+export const Changes: Story = {
+  render: () => {
+    return (
+      <div className="dark">
+        <VersionErrorsAndChanges
+          changes={{
+            nodes: changes,
+            total: changes.length,
+          }}
+          errors={{
+            nodes: [],
+            total: 0,
+          }}
+        />
+      </div>
+    );
+  },
+};
+
+export const Errors: Story = {
+  render: () => {
+    return (
+      <VersionErrorsAndChanges
+        errors={{
+          nodes: errors,
+          total: errors.length,
+        }}
+        changes={{
+          nodes: [],
+          total: 0,
+        }}
+      />
+    );
+  },
+};
+
+export const Both: Story = {
+  render: () => {
+    return (
+      <VersionErrorsAndChanges
+        errors={{
+          nodes: errors,
+          total: errors.length,
+        }}
+        changes={{
+          nodes: changes,
+          total: changes.length,
+        }}
+      />
+    );
+  },
+};