console: allow editing rest endpoints queries and misc ui improvements

Co-authored-by: Varun Choudhary <68095256+Varun-Choudhary@users.noreply.github.com>
Co-authored-by: Aleksandra Sikora <9019397+beerose@users.noreply.github.com>
GitOrigin-RevId: 4dd360eb07b836c6fd6ce666c9444728e22847d7

CHANGELOG.md

@@ -10,6 +10,7 @@
 
 - server: fix inherited_roles issue when some of the underlying roles don't have permissions configured (fixes #6672)
 - server: fix action custom types failing to parse when mutually recursive
+- console: allow editing rest endpoints queries and misc ui improvements
 - cli: match ordering of keys in project metadata files with server metadata
 
 ## v2.0.0-alpha.5

console/src/components/Services/ApiExplorer/Rest/Create/RequestViewer.tsx

@@ -7,15 +7,27 @@ import styles from '../RESTStyles.scss';
 
 type RequestViewerProps = {
   request: string;
+  onChangeQueryText?: (value: string) => void;
+  isEditable?: boolean;
 };
 
-const RequestViewer: React.FC<RequestViewerProps> = ({ request }) => (
+const RequestViewer: React.FC<RequestViewerProps> = ({
+  request,
+  isEditable = false,
+  onChangeQueryText,
+}) => (
   <div className={styles.request_viewer_layout}>
     <div className={styles.request_viewer_heading}>
       <label className={styles.form_input_label}>GraphQL Request</label>
       <ToolTip message="The request your endpoint will run. All variables will be mapped to REST endpoint variables." />
     </div>
-    <Editor readOnly mode="graphqlschema" value={request} height="200px" />
+    <Editor
+      readOnly={!isEditable}
+      mode="graphqlschema"
+      value={request}
+      height="200px"
+      onChange={onChangeQueryText ?? undefined}
+    />
   </div>
 );
 

console/src/components/Services/ApiExplorer/Rest/Create/index.tsx

@@ -1,4 +1,4 @@
-import React, { useReducer, useEffect, useState } from 'react';
+import React, { useReducer, useEffect, useRef } from 'react';
 import { RouteComponentProps } from 'react-router';
 import { connect, ConnectedProps } from 'react-redux';
 
@@ -47,7 +47,21 @@ const cleanUpState = (state: CreateEndpointState) => {
 };
 
 const isErroredState = (state: CreateEndpointState) => {
-  return !state.name || !state.url || !state.request || !state.methods.length;
+  const errorFields = [];
+  if (!state.name) {
+    errorFields.push('Name');
+  }
+  if (!state.url) {
+    errorFields.push('Location');
+  }
+  if (!state.request) {
+    errorFields.push('Request');
+  }
+  if (!state.methods.length) {
+    errorFields.push('Method');
+  }
+
+  return errorFields;
 };
 
 const createEndpointObject = (
@@ -87,7 +101,7 @@ const CreateEndpoint: React.FC<CreateEndpointProps> = ({
     createEndpointReducer,
     defaultState
   );
-  const [oldState, updateOldState] = useState<RestEndpointEntry | null>(null);
+  const oldState = useRef<RestEndpointEntry | null>(null);
   const isPageCreate = location.pathname === '/api/rest/create';
   // currentPageName will have a valid value, when it's on the edit page
   let currentPageName = '';
@@ -166,7 +180,7 @@ const CreateEndpoint: React.FC<CreateEndpointProps> = ({
       // Ideally, this also, should not be happening
       return;
     }
-    updateOldState(oldRestEndpointEntry);
+    oldState.current = oldRestEndpointEntry;
     updateEndpointName(oldRestEndpointEntry.name);
     updateEndpointComment(oldRestEndpointEntry?.comment ?? '');
     updateEndpointURL(oldRestEndpointEntry.url);
@@ -190,14 +204,19 @@ const CreateEndpoint: React.FC<CreateEndpointProps> = ({
 
   const onClickCreate = () => {
     const cleanedUpState = cleanUpState(inputState);
-    if (isErroredState(cleanedUpState)) {
+    const catchedError = isErroredState(cleanedUpState);
+    const errorLen = catchedError.length;
+    if (errorLen >= 1) {
+      const fieldText = `field${errorLen >= 2 ? 's' : ''}`;
+      const isAreText = `${errorLen >= 2 ? 'are' : 'is a'}`;
       showError(
         'Some required fields are empty',
-        'Name, Location, Request and Methods are required fields.'
+        `${catchedError.join(', ')} ${isAreText} required ${fieldText}.`
       );
       return;
     }
-    // NOTE: this is a case of extreme safe coding. Maybe be unecessary here.
+    // NOTE: this check is necessary for the edit page
+    // where queries can be edited
     if (!isQueryValid(cleanedUpState.request)) {
       showError(
         'Invalid Query being used to create endpoint',
@@ -209,10 +228,10 @@ const CreateEndpoint: React.FC<CreateEndpointProps> = ({
       createEndpoint(restEndpointObj, request, resetPageState);
       return;
     }
-    if (!oldState) {
+    if (!oldState || !oldState.current) {
       return;
     }
-    editEndpoint(oldState, restEndpointObj, request, resetPageState);
+    editEndpoint(oldState.current, restEndpointObj, request, resetPageState);
   };
 
   return (
@@ -254,7 +273,11 @@ const CreateEndpoint: React.FC<CreateEndpointProps> = ({
           currentState={inputState.methods}
           updateState={updateEndpointMethods}
         />
-        <RequestViewer request={inputState.request} />
+        <RequestViewer
+          request={inputState.request}
+          isEditable={!isPageCreate}
+          onChangeQueryText={isPageCreate ? undefined : updateEndpointRequest}
+        />
       </div>
       <div className={styles.rest_action_btns}>
         <Button color="white" onClick={resetPageState}>

console/src/components/Services/ApiExplorer/Rest/Create/state.ts

@@ -1,10 +1,10 @@
 import { AllowedRESTMethods } from '../../../../../metadata/types';
 
 type CreateEndpointState = {
-  name: string;
-  comment: string;
-  url: string;
-  methods: AllowedRESTMethods[];
+  name: string; // Name on UI
+  comment: string; // Description on UI
+  url: string; // Location on UI
+  methods: AllowedRESTMethods[]; // Methods on the UI
   request: string;
 };
 

console/src/components/Services/ApiExplorer/Rest/Landing.tsx

@@ -6,6 +6,10 @@ import LandingImage from './LandingImage';
 
 import styles from './RESTStyles.scss';
 
+const landingDescription = `REST endpoints allow for the creation of a REST interface to your saved GraphQL queries and mutations.
+Endpoints are accessible from /api/rest/* and inherit the authorization and permission structure from your associated GraphQL nodes. 
+To create a new endpoint simply test your query in GraphiQL then click the REST button on GraphiQL to configure a URL.`;
+
 const Landing = () => (
   <div
     className={`container-fluid ${styles.rest_add_padding_left} ${styles.padd_top}`}
@@ -26,7 +30,7 @@ const Landing = () => (
       title="What are REST endpoints?"
       imgElement={<LandingImage />}
       imgAlt="REST endpoints"
-      description="REST endpoints allow for the creation of a REST interface to your saved GraphQL queries and mutations. Endpoints are generated from /api/rest/* and inherit the authorization and permission structure from your associated GraphQL nodes."
+      description={landingDescription}
       knowMoreHref="https://hasura.io/docs/latest/graphql/core/api-reference/restified.html"
     />
     <hr className={styles.clear_fix} />

console/src/components/Services/ApiExplorer/Rest/utils.ts

@@ -33,14 +33,19 @@ export const badgeSort = (methods: AllowedRESTMethods[]) => {
     .map(method => method.name);
 };
 
-// checkIfSubscription is a temporary method being added to prevent endpoints
+// checkIfSubscription is a method being added to prevent endpoints
 // with subscriptions being created. See Issue (#628)
 // using `any` here since 'operation' is not defined on the type DefinitionNode
 const checkIfSubscription = (queryRootNode: any) => {
   return queryRootNode?.operation === 'subscription';
 };
 
-// checkValidQuery is a helper to validate a query that's being created
+const checkIfAnonymousQuery = (queryRootNode: any) => {
+  // It probably doesn't have to be this explicit
+  return queryRootNode?.name === undefined;
+};
+
+// isQueryValid is a helper to validate a query that's being created
 // this helps avoiding to creating endpoints with comments, empty spaces
 // and queries that are subscriptions (see above comment for reference)
 export const isQueryValid = (query: string) => {
@@ -50,7 +55,20 @@ export const isQueryValid = (query: string) => {
 
   try {
     const parsedAST = parse(query);
-    return !checkIfSubscription(parsedAST.definitions[0]);
+    if (!parsedAST) {
+      return false;
+    }
+    // making sure that there's only 1 definition in the query
+    // query shouldn't be a subscription - server also throws an error for the same
+    // also a check's in place to make sure that the query is named
+    if (
+      parsedAST?.definitions?.length > 1 ||
+      checkIfSubscription(parsedAST.definitions[0]) ||
+      checkIfAnonymousQuery(parsedAST.definitions[0])
+    ) {
+      return false;
+    }
+    return true;
   } catch {
     return false;
   }

console/src/metadata/actions.ts

@@ -166,15 +166,6 @@ export interface UpdateInheritedRole {
     role_set: string[];
   };
 }
-export interface AddRestEndpoint {
-  type: 'Metadata/ADD_REST_ENDPOINT';
-  data: RestEndpointEntry[];
-}
-
-export interface DropRestEndpoint {
-  type: 'Metadata/DROP_REST_ENDPOINT';
-  data: RestEndpointEntry[];
-}
 
 export type MetadataActions =
   | ExportMetadataSuccess
@@ -200,8 +191,6 @@ export type MetadataActions =
   | AddInheritedRole
   | DeleteInheritedRole
   | UpdateInheritedRole
-  | AddRestEndpoint
-  | DropRestEndpoint
   | { type: typeof UPDATE_CURRENT_DATA_SOURCE; source: string };
 
 export const exportMetadata = (
@@ -987,11 +976,7 @@ export const addRESTEndpoint = (
   getState
 ) => {
   const { currentDataSource } = getState().tables;
-  const { rest_endpoints, metadataObject } = getState().metadata;
-  let currentEndpoints: RestEndpointEntry[] = [];
-  if (rest_endpoints?.length) {
-    currentEndpoints = rest_endpoints;
-  }
+  const { metadataObject } = getState().metadata;
   const upQueries = [];
   const downQueries = [];
 
@@ -1025,10 +1010,6 @@ export const addRESTEndpoint = (
   const errorMsg = 'Error creating REST endpoint';
 
   const onSuccess = () => {
-    dispatch({
-      type: 'Metadata/ADD_REST_ENDPOINT',
-      data: [...currentEndpoints, queryObj],
-    });
     if (successCb) {
       successCb();
     }
@@ -1088,10 +1069,6 @@ export const dropRESTEndpoint = (
     return;
   }
 
-  const filteredEndpoints = currentRESTEndpoints.filter(
-    re => re.name !== endpointName
-  );
-
   const confirmation = getConfirmation(
     `You want to delete the endpoint: ${endpointName}`
   );
@@ -1115,10 +1092,6 @@ export const dropRESTEndpoint = (
   const errorMsg = 'Error dropping REST endpoint';
 
   const onSuccess = () => {
-    dispatch({
-      type: 'Metadata/DROP_REST_ENDPOINT',
-      data: filteredEndpoints,
-    });
     if (successCb) {
       successCb();
     }
@@ -1155,6 +1128,7 @@ export const editRESTEndpoint = (
   getState
 ) => {
   const currentEndpoints = getState().metadata.metadataObject?.rest_endpoints;
+
   if (!currentEndpoints) {
     dispatch(
       showErrorNotification(
@@ -1165,56 +1139,38 @@ export const editRESTEndpoint = (
     return;
   }
 
-  // using `any` here since the 3 queries have a different
-  // return types, hence ended up using any
-  let upQueries: any = [
+  const dropOldQueryFromCollection = deleteAllowedQueryQuery(oldQueryObj.name);
+  const addNewQueryToCollection = addAllowedQuery({
+    name: newQueryObj.name,
+    query: request,
+  });
+
+  const dropNewQueryFromCollection = deleteAllowedQueryQuery(newQueryObj.name);
+  const addOldQueryToCollection = addAllowedQuery({
+    name: oldQueryObj.name,
+    query: request,
+  });
+
+  const upQueries = [
     dropRESTEndpointQuery(oldQueryObj.name),
+    dropOldQueryFromCollection,
+    addNewQueryToCollection,
     createRESTEndpointQuery(newQueryObj),
   ];
-  let downQueries: any = [
-    createRESTEndpointQuery(oldQueryObj),
-    dropRESTEndpointQuery(newQueryObj.name),
-  ];
 
-  if (newQueryObj.name !== oldQueryObj.name) {
-    const newAllowedQuery = addAllowedQuery({
-      name: newQueryObj.name,
-      query: request,
-    });
-    const deleteOldFromCollection = deleteAllowedQueryQuery(oldQueryObj.name);
-    const addOldIntoQueryCollection = addAllowedQuery({
-      name: oldQueryObj.name,
-      query: request,
-    });
-    const newDeleteAllowedQuery = deleteAllowedQueryQuery(newQueryObj.name);
-    upQueries = [
-      upQueries[0],
-      deleteOldFromCollection,
-      newAllowedQuery,
-      upQueries[1],
-    ];
-    downQueries = [
-      addOldIntoQueryCollection,
-      downQueries[0],
-      downQueries[1],
-      newDeleteAllowedQuery,
-    ];
-  }
+  const downQueries = [
+    dropRESTEndpointQuery(newQueryObj.name),
+    dropNewQueryFromCollection,
+    addOldQueryToCollection,
+    createRESTEndpointQuery(oldQueryObj),
+  ];
 
   const migrationName = `edit_rest_endpoint_${newQueryObj.url}_${newQueryObj.name}`;
   const requestMsg = `Editing REST endpoint ${oldQueryObj.name}`;
   const successMsg = 'Successfully edited REST endpoint';
   const errorMsg = 'Error editing REST endpoint';
 
-  const filteredEndpoints = currentEndpoints.filter(
-    re => re.name !== oldQueryObj.name
-  );
-
   const onSuccess = () => {
-    dispatch({
-      type: 'Metadata/ADD_REST_ENDPOINT',
-      data: [...filteredEndpoints, newQueryObj],
-    });
     if (successCb) {
       successCb();
     }

console/src/metadata/reducer.ts

@@ -1,10 +1,5 @@
 import { MetadataActions } from './actions';
-import {
-  QueryCollection,
-  HasuraMetadataV3,
-  RestEndpointEntry,
-  InheritedRole,
-} from './types';
+import { QueryCollection, HasuraMetadataV3, InheritedRole } from './types';
 import { allowedQueriesCollection } from './utils';
 
 type MetadataState = {
@@ -15,7 +10,6 @@ type MetadataState = {
   ongoingRequest: boolean; // deprecate
   allowedQueries: QueryCollection[];
   inheritedRoles: InheritedRole[];
-  rest_endpoints?: RestEndpointEntry[];
 };
 
 const defaultState: MetadataState = {
@@ -145,17 +139,6 @@ export const metadataReducer = (
           ),
         ],
       };
-
-    case 'Metadata/ADD_REST_ENDPOINT':
-      return {
-        ...state,
-        rest_endpoints: action.data,
-      };
-    case 'Metadata/DROP_REST_ENDPOINT':
-      return {
-        ...state,
-        rest_endpoints: action.data,
-      };
     default:
       return state;
   }