RI-7186 validate job name separately

redisinsight/ui/src/components/yaml-validator/validatePipeline.test.ts

@@ -1,6 +1,6 @@
 import { get } from 'lodash'
 import { validatePipeline } from './validatePipeline'
-import { validateYamlSchema } from './validateYamlSchema'
+import { validateYamlSchema, validateSchema } from './validateYamlSchema'
 
 jest.mock('./validateYamlSchema')
 
@@ -31,10 +31,16 @@ describe('validatePipeline', () => {
       valid: true,
       errors: [],
     }))
+    ;(validateSchema as jest.Mock).mockImplementation(() => ({
+      valid: true,
+      errors: [],
+    }))
 
     const result = validatePipeline({
       config: 'name: valid-config',
       schema: mockSchema,
+      monacoJobsSchema: null,
+      jobNameSchema: null,
       jobs: [
         { name: 'Job1', value: 'task: job1' },
         { name: 'Job2', value: 'task: job2' },
@@ -57,10 +63,16 @@ describe('validatePipeline', () => {
         ? { valid: false, errors: ["Missing required property 'name'"] }
         : { valid: true, errors: [] },
     )
+    ;(validateSchema as jest.Mock).mockImplementation(() => ({
+      valid: true,
+      errors: [],
+    }))
 
     const result = validatePipeline({
       config: 'invalid-config-content',
       schema: mockSchema,
+      monacoJobsSchema: null,
+      jobNameSchema: null,
       jobs: [{ name: 'Job1', value: 'task: job1' }],
     })
 
@@ -75,14 +87,20 @@ describe('validatePipeline', () => {
 
   it('should return invalid result when jobs are invalid', () => {
     ;(validateYamlSchema as jest.Mock).mockImplementation((_, schema) =>
-      schema === get(mockSchema, 'jobs', null)
+      schema === null
         ? { valid: false, errors: ["Missing required property 'task'"] }
         : { valid: true, errors: [] },
     )
+    ;(validateSchema as jest.Mock).mockImplementation(() => ({
+      valid: true,
+      errors: [],
+    }))
 
     const result = validatePipeline({
       config: 'name: valid-config',
       schema: mockSchema,
+      monacoJobsSchema: null,
+      jobNameSchema: null,
       jobs: [{ name: 'Job1', value: 'invalid-job-content' }],
     })
 
@@ -100,15 +118,21 @@ describe('validatePipeline', () => {
       if (schema === get(mockSchema, 'config', null)) {
         return { valid: false, errors: ["Missing required property 'name'"] }
       }
-      if (schema === get(mockSchema, 'jobs', null)) {
+      if (schema === null) {
         return { valid: false, errors: ["Missing required property 'task'"] }
       }
       return { valid: true, errors: [] }
     })
+    ;(validateSchema as jest.Mock).mockImplementation(() => ({
+      valid: true,
+      errors: [],
+    }))
 
     const result = validatePipeline({
       config: 'invalid-config-content',
       schema: mockSchema,
+      monacoJobsSchema: null,
+      jobNameSchema: null,
       jobs: [{ name: 'Job1', value: 'invalid-job-content' }],
     })
 
@@ -126,10 +150,16 @@ describe('validatePipeline', () => {
       valid: false,
       errors: ['Duplicate error', 'Duplicate error'], // all the jobs get these errors
     }))
+    ;(validateSchema as jest.Mock).mockImplementation(() => ({
+      valid: true,
+      errors: [],
+    }))
 
     const result = validatePipeline({
       config: 'invalid-config-content',
       schema: mockSchema,
+      monacoJobsSchema: null,
+      jobNameSchema: null,
       jobs: [
         { name: 'Job1', value: 'invalid-job-content' },
         { name: 'Job2', value: 'invalid-job-content' },
@@ -145,4 +175,31 @@ describe('validatePipeline', () => {
       },
     })
   })
+
+  it('should return invalid result when job name validation fails', () => {
+    ;(validateYamlSchema as jest.Mock).mockImplementation(() => ({
+      valid: true,
+      errors: [],
+    }))
+    ;(validateSchema as jest.Mock).mockImplementation(() => ({
+      valid: false,
+      errors: ['Job name: Invalid job name'],
+    }))
+
+    const result = validatePipeline({
+      config: 'name: valid-config',
+      schema: mockSchema,
+      monacoJobsSchema: null,
+      jobNameSchema: { type: 'string', pattern: '^[a-zA-Z]+$' },
+      jobs: [{ name: 'Job-1', value: 'task: job1' }],
+    })
+
+    expect(result).toEqual({
+      result: false,
+      configValidationErrors: [],
+      jobsValidationErrors: {
+        'Job-1': ['Job name: Invalid job name'],
+      },
+    })
+  })
 })

redisinsight/ui/src/components/yaml-validator/validatePipeline.ts

@@ -1,15 +1,20 @@
 import { get } from 'lodash'
-import { validateYamlSchema } from './validateYamlSchema'
+import { Nullable } from 'uiSrc/utils'
+import { validateSchema, validateYamlSchema } from './validateYamlSchema'
 
 interface PipelineValidationProps {
   config: string
   schema: any
+  monacoJobsSchema: Nullable<object>
+  jobNameSchema: Nullable<object>
   jobs: { name: string; value: string }[]
 }
 
 export const validatePipeline = ({
   config,
   schema,
+  monacoJobsSchema,
+  jobNameSchema,
   jobs,
 }: PipelineValidationProps) => {
   const { valid: isConfigValid, errors: configErrors } = validateYamlSchema(
@@ -22,7 +27,11 @@ export const validatePipeline = ({
     jobsErrors: Record<string, Set<string>>
   }>(
     (acc, j) => {
-      const validation = validateYamlSchema(j.value, get(schema, 'jobs', null))
+      const validation = validateYamlSchema(j.value, monacoJobsSchema)
+      const jobNameValidation = validateSchema(j.name, jobNameSchema, {
+        errorMessagePrefix: 'Job name',
+        includePathIntoErrorMessage: false,
+      })
 
       if (!acc.jobsErrors[j.name]) {
         acc.jobsErrors[j.name] = new Set()
@@ -32,7 +41,14 @@ export const validatePipeline = ({
         validation.errors.forEach((error) => acc.jobsErrors[j.name].add(error))
       }
 
-      acc.areJobsValid = acc.areJobsValid && validation.valid
+      if (!jobNameValidation.valid) {
+        jobNameValidation.errors.forEach((error) =>
+          acc.jobsErrors[j.name].add(error),
+        )
+      }
+
+      acc.areJobsValid =
+        acc.areJobsValid && validation.valid && jobNameValidation.valid
       return acc
     },
     { areJobsValid: true, jobsErrors: {} },

redisinsight/ui/src/components/yaml-validator/validateYamlSchema.test.ts

@@ -1,5 +1,5 @@
 import yaml from 'js-yaml'
-import { validateYamlSchema } from './validateYamlSchema'
+import { validateYamlSchema, validateSchema } from './validateYamlSchema'
 
 const schema = {
   type: 'object',
@@ -97,3 +97,168 @@ describe('validateYamlSchema', () => {
     jest.restoreAllMocks()
   })
 })
+
+describe('validateSchema with ValidationConfig', () => {
+  const testSchema = {
+    type: 'object',
+    properties: {
+      name: { type: 'string' },
+      nested: {
+        type: 'object',
+        properties: {
+          value: { type: 'number' }
+        },
+        required: ['value']
+      }
+    },
+    required: ['name']
+  }
+
+  const invalidData = {
+    nested: {
+      value: 'not-a-number'
+    }
+    // missing required 'name' field
+  }
+
+  describe('default ValidationConfig', () => {
+    it('should use default error message prefix "Error:"', () => {
+      const result = validateSchema(invalidData, testSchema)
+
+      expect(result.valid).toBe(false)
+      expect(result.errors).toEqual(
+        expect.arrayContaining([
+          expect.stringContaining('Error:')
+        ])
+      )
+    })
+
+    it('should include path information by default', () => {
+      const result = validateSchema(invalidData, testSchema)
+
+      expect(result.valid).toBe(false)
+      expect(result.errors).toEqual(
+        expect.arrayContaining([
+          expect.stringContaining('(at root)'),
+          expect.stringContaining('(at /nested/value)')
+        ])
+      )
+    })
+  })
+
+  describe('custom ValidationConfig', () => {
+    it('should use custom error message prefix', () => {
+      const config = { errorMessagePrefix: 'Custom Prefix:' }
+      const result = validateSchema(invalidData, testSchema, config)
+
+      expect(result.valid).toBe(false)
+      expect(result.errors).toEqual(
+        expect.arrayContaining([
+          expect.stringContaining('Custom Prefix:')
+        ])
+      )
+      expect(result.errors).not.toEqual(
+        expect.arrayContaining([
+          expect.stringContaining('Error:')
+        ])
+      )
+    })
+
+    it('should exclude path information when includePathIntoErrorMessage is false', () => {
+      const config = { includePathIntoErrorMessage: false }
+      const result = validateSchema(invalidData, testSchema, config)
+
+      expect(result.valid).toBe(false)
+      expect(result.errors).not.toEqual(
+        expect.arrayContaining([
+          expect.stringContaining('(at ')
+        ])
+      )
+    })
+
+    it('should use both custom prefix and exclude path information', () => {
+      const config = { 
+        errorMessagePrefix: 'Custom Error:', 
+        includePathIntoErrorMessage: false 
+      }
+      const result = validateSchema(invalidData, testSchema, config)
+
+      expect(result.valid).toBe(false)
+      expect(result.errors).toEqual(
+        expect.arrayContaining([
+          expect.stringContaining('Custom Error:')
+        ])
+      )
+      expect(result.errors).not.toEqual(
+        expect.arrayContaining([
+          expect.stringContaining('(at ')
+        ])
+      )
+    })
+
+    it('should handle empty string as error message prefix', () => {
+      const config = { errorMessagePrefix: '' }
+      const result = validateSchema(invalidData, testSchema, config)
+
+      expect(result.valid).toBe(false)
+      expect(result.errors.length).toBeGreaterThan(0)
+      // Should not start with "Error:" but with the actual error message
+      expect(result.errors[0]).not.toMatch(/^Error:/)
+    })
+  })
+
+  describe('ValidationConfig with exceptions', () => {
+    it('should use custom error prefix for unknown errors', () => {
+      const mockSchema = null // This will cause an error in AJV
+      const config = { errorMessagePrefix: 'Schema Error:' }
+
+      const result = validateSchema({}, mockSchema, config)
+
+      expect(result.valid).toBe(false)
+      expect(result.errors).toEqual(['Schema Error: unknown error'])
+    })
+
+    it('should use default error prefix for unknown errors when no config provided', () => {
+      const mockSchema = null // This will cause an error in AJV
+
+      const result = validateSchema({}, mockSchema)
+
+      expect(result.valid).toBe(false)
+      expect(result.errors).toEqual(['Error: unknown error'])
+    })
+  })
+
+  describe('edge cases', () => {
+    it('should handle valid data with custom config', () => {
+      const validData = { name: 'test', nested: { value: 42 } }
+      const config = { 
+        errorMessagePrefix: 'Custom Error:', 
+        includePathIntoErrorMessage: false 
+      }
+
+      const result = validateSchema(validData, testSchema, config)
+
+      expect(result).toEqual({
+        valid: true,
+        errors: []
+      })
+    })
+
+    it('should handle undefined config properties gracefully', () => {
+      const config = { 
+        errorMessagePrefix: undefined, 
+        includePathIntoErrorMessage: undefined 
+      }
+      const result = validateSchema(invalidData, testSchema, config)
+
+      expect(result.valid).toBe(false)
+      // Should use defaults when undefined
+      expect(result.errors).toEqual(
+        expect.arrayContaining([
+          expect.stringContaining('Error:'),
+          expect.stringContaining('(at ')
+        ])
+      )
+    })
+  })
+})