diff --git a/docs/openapi/api.yaml b/docs/openapi/api.yaml index 3f9086dfb..d5b7fa5fe 100644 --- a/docs/openapi/api.yaml +++ b/docs/openapi/api.yaml @@ -314,10 +314,6 @@ paths: $ref: './llmo-api.yaml#/llmo-global-sheet-data' /sites/{siteId}/llmo/config: $ref: './llmo-api.yaml#/llmo-config' - /sites/{siteId}/llmo/questions: - $ref: './llmo-api.yaml#/llmo-questions' - /sites/{siteId}/llmo/questions/{questionKey}: - $ref: './llmo-api.yaml#/llmo-question' /sites/{siteId}/llmo/customer-intent: $ref: './llmo-api.yaml#/llmo-customer-intent' /sites/{siteId}/llmo/customer-intent/{intentKey}: diff --git a/docs/openapi/llmo-api.yaml b/docs/openapi/llmo-api.yaml index c5833ea85..3b44615d4 100644 --- a/docs/openapi/llmo-api.yaml +++ b/docs/openapi/llmo-api.yaml @@ -895,126 +895,6 @@ llmo-offboard: security: - api_key: [ ] -llmo-questions: - parameters: - - $ref: './parameters.yaml#/siteId' - get: - tags: - - llmo - summary: Get LLMO questions - description: | - Retrieves all LLMO questions (both human and AI) for a specific site. - Returns an object with Human and AI question arrays. - operationId: getLlmoQuestions - responses: - '200': - description: LLMO questions retrieved successfully - content: - application/json: - schema: - $ref: './schemas.yaml#/LlmoQuestions' - '400': - $ref: './responses.yaml#/400' - '401': - $ref: './responses.yaml#/401' - '500': - $ref: './responses.yaml#/500' - security: - - api_key: [ ] - post: - tags: - - llmo - summary: Add LLMO questions - description: | - Adds new questions to the LLMO configuration for a specific site. - Questions can be added to both Human and AI categories. - operationId: addLlmoQuestion - requestBody: - required: true - content: - application/json: - schema: - $ref: './schemas.yaml#/LlmoQuestionsInput' - responses: - '200': - description: LLMO questions added successfully - content: - application/json: - schema: - $ref: './schemas.yaml#/LlmoQuestions' - '400': - $ref: './responses.yaml#/400' - '401': - $ref: './responses.yaml#/401' - '500': - $ref: './responses.yaml#/500' - security: - - api_key: [ ] - -llmo-question: - parameters: - - $ref: './parameters.yaml#/siteId' - - name: questionKey - in: path - required: true - description: The unique key of the question to modify - schema: - type: string - format: uuid - example: '123e4567-e89b-12d3-a456-426614174000' - delete: - tags: - - llmo - summary: Remove LLMO question - description: | - Removes a specific question from the LLMO configuration by its unique key. - The question can be from either Human or AI categories. - operationId: removeLlmoQuestion - responses: - '200': - description: LLMO question removed successfully - content: - application/json: - schema: - $ref: './schemas.yaml#/LlmoQuestions' - '400': - $ref: './responses.yaml#/400' - '401': - $ref: './responses.yaml#/401' - '500': - $ref: './responses.yaml#/500' - security: - - api_key: [ ] - patch: - tags: - - llmo - summary: Update LLMO question - description: | - Updates a specific question in the LLMO configuration by its unique key. - The question can be from either Human or AI categories. - operationId: patchLlmoQuestion - requestBody: - required: true - content: - application/json: - schema: - $ref: './schemas.yaml#/LlmoQuestionUpdate' - responses: - '200': - description: LLMO question updated successfully - content: - application/json: - schema: - $ref: './schemas.yaml#/LlmoQuestions' - '400': - $ref: './responses.yaml#/400' - '401': - $ref: './responses.yaml#/401' - '500': - $ref: './responses.yaml#/500' - security: - - api_key: [ ] - llmo-customer-intent: parameters: - $ref: './parameters.yaml#/siteId' diff --git a/src/controllers/llmo/llmo.js b/src/controllers/llmo/llmo.js index 005498335..676e22c87 100644 --- a/src/controllers/llmo/llmo.js +++ b/src/controllers/llmo/llmo.js @@ -23,7 +23,6 @@ import { composeBaseURL, } from '@adobe/spacecat-shared-utils'; import { Config } from '@adobe/spacecat-shared-data-access/src/models/site/config.js'; -import crypto from 'crypto'; import { Entitlement as EntitlementModel } from '@adobe/spacecat-shared-data-access'; import AccessControlUtil from '../../support/access-control-util.js'; import { triggerBrandProfileAgent } from '../../support/brand-profile-trigger.js'; @@ -84,17 +83,6 @@ function LlmoController(ctx) { } }; - // Helper function to validate question key - const validateQuestionKey = (config, questionKey) => { - const humanQuestions = config.getLlmoHumanQuestions() || []; - const aiQuestions = config.getLlmoAIQuestions() || []; - - if (!humanQuestions.some((question) => question.key === questionKey) - && !aiQuestions.some((question) => question.key === questionKey)) { - throw new Error('Invalid question key, please provide a valid question key'); - } - }; - // Helper function to validate customer intent key const validateCustomerIntentKey = (config, intentKey) => { const customerIntent = config.getLlmoCustomerIntent() || []; @@ -509,88 +497,6 @@ function LlmoController(ctx) { } } - // Handles requests to the LLMO questions endpoint, returns both human and ai questions - const getLlmoQuestions = async (context) => { - const { llmoConfig } = await getSiteAndValidateLlmo(context); - return ok(llmoConfig.questions || {}); - }; - - // Handles requests to the LLMO questions endpoint, adds a new question - // the body format is { Human: [question1, question2], AI: [question3, question4] } - const addLlmoQuestion = async (context) => { - const { log } = context; - const { site, config } = await getSiteAndValidateLlmo(context); - - // add the question to the llmoConfig - const newQuestions = context.data; - if (!newQuestions) { - return badRequest('No questions provided in the request body'); - } - let updated = false; - - // Prepare human questions with unique keys - if (newQuestions.Human && newQuestions.Human.length > 0) { - const humanQuestionsWithKeys = newQuestions.Human.map((question) => ({ - ...question, - key: crypto.randomUUID(), - })); - config.addLlmoHumanQuestions(humanQuestionsWithKeys); - updated = true; - } - - // Prepare AI questions with unique keys - if (newQuestions.AI && newQuestions.AI.length > 0) { - const aiQuestionsWithKeys = newQuestions.AI.map((question) => ({ - ...question, - key: crypto.randomUUID(), - })); - config.addLlmoAIQuestions(aiQuestionsWithKeys); - updated = true; - } - - if (updated) { - await saveSiteConfig(site, config, log, 'adding new questions'); - } - - // return the updated llmoConfig questions - return ok(config.getLlmoConfig().questions); - }; - - // Handles requests to the LLMO questions endpoint, removes a question - const removeLlmoQuestion = async (context) => { - const { log } = context; - const { questionKey } = context.params; - const { site, config } = await getSiteAndValidateLlmo(context); - - validateQuestionKey(config, questionKey); - - // remove the question using the config method - config.removeLlmoQuestion(questionKey); - - await saveSiteConfig(site, config, log, 'removing question'); - - // return the updated llmoConfig questions - return ok(config.getLlmoConfig().questions); - }; - - // Handles requests to the LLMO questions endpoint, updates a question - const patchLlmoQuestion = async (context) => { - const { log } = context; - const { questionKey } = context.params; - const { data } = context; - const { site, config } = await getSiteAndValidateLlmo(context); - - validateQuestionKey(config, questionKey); - - // update the question using the config method - config.updateLlmoQuestion(questionKey, data); - - await saveSiteConfig(site, config, log, 'updating question'); - - // return the updated llmoConfig questions - return ok(config.getLlmoConfig().questions); - }; - // Handles requests to the LLMO customer intent endpoint, returns customer intent array const getLlmoCustomerIntent = async (context) => { try { @@ -910,10 +816,6 @@ function LlmoController(ctx) { queryLlmoSheetData, getLlmoGlobalSheetData, getLlmoConfig, - getLlmoQuestions, - addLlmoQuestion, - removeLlmoQuestion, - patchLlmoQuestion, getLlmoCustomerIntent, addLlmoCustomerIntent, removeLlmoCustomerIntent, diff --git a/src/routes/index.js b/src/routes/index.js index eed6b64c4..09c6d4c58 100644 --- a/src/routes/index.js +++ b/src/routes/index.js @@ -320,10 +320,6 @@ export default function getRouteHandlers( 'GET /sites/:siteId/llmo/config': llmoController.getLlmoConfig, 'PATCH /sites/:siteId/llmo/config': llmoController.updateLlmoConfig, 'POST /sites/:siteId/llmo/config': llmoController.updateLlmoConfig, - 'GET /sites/:siteId/llmo/questions': llmoController.getLlmoQuestions, - 'POST /sites/:siteId/llmo/questions': llmoController.addLlmoQuestion, - 'DELETE /sites/:siteId/llmo/questions/:questionKey': llmoController.removeLlmoQuestion, - 'PATCH /sites/:siteId/llmo/questions/:questionKey': llmoController.patchLlmoQuestion, 'GET /sites/:siteId/llmo/customer-intent': llmoController.getLlmoCustomerIntent, 'POST /sites/:siteId/llmo/customer-intent': llmoController.addLlmoCustomerIntent, 'DELETE /sites/:siteId/llmo/customer-intent/:intentKey': llmoController.removeLlmoCustomerIntent, diff --git a/test/controllers/llmo/llmo.test.js b/test/controllers/llmo/llmo.test.js index 4eef276c7..d698c8fbc 100644 --- a/test/controllers/llmo/llmo.test.js +++ b/test/controllers/llmo/llmo.test.js @@ -128,10 +128,6 @@ describe('LlmoController', () => { mockLlmoConfig = { dataFolder: TEST_FOLDER, brand: TEST_BRAND, - questions: { - Human: [{ key: 'test-question', question: 'What is the main goal of this page?' }], - AI: [{ key: 'ai-question', question: 'Analyze the page content and identify key themes.' }], - }, customerIntent: [ { key: 'target_audience', value: 'small business owners' }, { key: 'primary_goal', value: 'increase conversions' }, @@ -152,12 +148,6 @@ describe('LlmoController', () => { mockConfig = { getLlmoConfig: sinon.stub().returns(mockLlmoConfig), updateLlmoConfig: sinon.stub(), - addLlmoHumanQuestions: sinon.stub(), - addLlmoAIQuestions: sinon.stub(), - removeLlmoQuestion: sinon.stub(), - updateLlmoQuestion: sinon.stub(), - getLlmoHumanQuestions: sinon.stub().returns(mockLlmoConfig.questions.Human), - getLlmoAIQuestions: sinon.stub().returns(mockLlmoConfig.questions.AI), getLlmoCustomerIntent: sinon.stub().returns(mockLlmoConfig.customerIntent), addLlmoCustomerIntent: sinon.stub(), removeLlmoCustomerIntent: sinon.stub(), @@ -247,12 +237,8 @@ describe('LlmoController', () => { siteId: TEST_SITE_ID, dataSource: 'test-data', configName: 'test-data', - questionKey: 'test-question', - }, - data: { - Human: [{ question: 'New human question?' }], - AI: [{ question: 'New AI question?' }], }, + data: {}, dataAccess: mockDataAccess, log: mockLog, env: mockEnv, @@ -1499,115 +1485,6 @@ describe('LlmoController', () => { }); }); - describe('getLlmoQuestions', () => { - it('should return LLMO questions successfully', async () => { - const result = await controller.getLlmoQuestions(mockContext); - - expect(result.status).to.equal(200); - const responseBody = await result.json(); - expect(responseBody).to.deep.equal(mockLlmoConfig.questions); - }); - - it('should return empty questions when not configured', async () => { - mockConfig.getLlmoConfig.returns({ dataFolder: TEST_FOLDER, brand: TEST_BRAND }); - - const result = await controller.getLlmoQuestions(mockContext); - - expect(result.status).to.equal(200); - const responseBody = await result.json(); - expect(responseBody).to.deep.equal({}); - }); - }); - - describe('addLlmoQuestion', () => { - it('should add human questions successfully', async () => { - const result = await controller.addLlmoQuestion(mockContext); - - expect(result.status).to.equal(200); - expect(mockConfig.addLlmoHumanQuestions).to.have.been.calledOnce; - expect(mockSite.save).to.have.been.calledOnce; - }); - - it('should add AI questions successfully', async () => { - mockContext.data = { AI: [{ question: 'New AI question?' }] }; - - const result = await controller.addLlmoQuestion(mockContext); - - expect(result.status).to.equal(200); - expect(mockConfig.addLlmoAIQuestions).to.have.been.calledOnce; - }); - - it('should return bad request when no questions provided', async () => { - mockContext.data = null; - - const result = await controller.addLlmoQuestion(mockContext); - - expect(result.status).to.equal(400); - }); - - it('should not save when no questions are added', async () => { - mockContext.data = {}; - - await controller.addLlmoQuestion(mockContext); - - expect(mockSite.save).to.not.have.been.called; - }); - - it('should handle save errors gracefully', async () => { - mockSite.save.rejects(new Error('Database connection failed')); - - const result = await controller.addLlmoQuestion(mockContext); - - expect(result.status).to.equal(200); - expect(mockLog.error).to.have.been.called; - }); - }); - - describe('removeLlmoQuestion', () => { - it('should remove question successfully', async () => { - const result = await controller.removeLlmoQuestion(mockContext); - - expect(result.status).to.equal(200); - expect(mockConfig.removeLlmoQuestion).to.have.been.calledWith('test-question'); - }); - - it('should throw error for invalid question key', async () => { - mockContext.params.questionKey = 'invalid-key'; - - try { - await controller.removeLlmoQuestion(mockContext); - expect.fail('Should have thrown an error'); - } catch (error) { - expect(error.message).to.include('Invalid question key'); - } - }); - - it('should handle null human and AI questions gracefully', async () => { - mockConfig.getLlmoHumanQuestions.returns(null); - mockConfig.getLlmoAIQuestions.returns(null); - mockContext.params.questionKey = 'any-key'; - - try { - await controller.removeLlmoQuestion(mockContext); - expect.fail('Should have thrown an error'); - } catch (error) { - expect(error.message).to.include('Invalid question key'); - } - }); - }); - - describe('patchLlmoQuestion', () => { - it('should update question successfully', async () => { - const updateData = { question: 'Updated question?' }; - mockContext.data = updateData; - - const result = await controller.patchLlmoQuestion(mockContext); - - expect(result.status).to.equal(200); - expect(mockConfig.updateLlmoQuestion).to.have.been.calledWith('test-question', updateData); - }); - }); - describe('getLlmoCustomerIntent', () => { it('should return LLMO customer intent successfully', async () => { const result = await controller.getLlmoCustomerIntent(mockContext); @@ -1742,6 +1619,18 @@ describe('LlmoController', () => { expect(result.status).to.equal(400); }); + + it('should handle save errors gracefully and still return success', async () => { + mockContext.data = [{ key: 'new_target', value: 'enterprise customers' }]; + mockSite.save.rejects(new Error('Database save failed')); + + const result = await controller.addLlmoCustomerIntent(mockContext); + + expect(result.status).to.equal(200); + expect(mockLog.error).to.have.been.calledWith( + sinon.match(/Error adding customer intent for site's llmo config/), + ); + }); }); describe('removeLlmoCustomerIntent', () => { diff --git a/test/routes/index.test.js b/test/routes/index.test.js index 80b6dabb1..35d26ac86 100755 --- a/test/routes/index.test.js +++ b/test/routes/index.test.js @@ -231,10 +231,6 @@ describe('getRouteHandlers', () => { getLlmoSheetData: () => null, getLlmoGlobalSheetData: () => null, getLlmoConfig: () => null, - getLlmoQuestions: () => null, - addLlmoQuestion: () => null, - removeLlmoQuestion: () => null, - patchLlmoQuestion: () => null, getLlmoCustomerIntent: () => null, addLlmoCustomerIntent: () => null, removeLlmoCustomerIntent: () => null, @@ -544,10 +540,6 @@ describe('getRouteHandlers', () => { 'GET /sites/:siteId/llmo/config', 'PATCH /sites/:siteId/llmo/config', 'POST /sites/:siteId/llmo/config', - 'GET /sites/:siteId/llmo/questions', - 'POST /sites/:siteId/llmo/questions', - 'DELETE /sites/:siteId/llmo/questions/:questionKey', - 'PATCH /sites/:siteId/llmo/questions/:questionKey', 'GET /sites/:siteId/llmo/customer-intent', 'POST /sites/:siteId/llmo/customer-intent', 'DELETE /sites/:siteId/llmo/customer-intent/:intentKey', @@ -711,14 +703,6 @@ describe('getRouteHandlers', () => { expect(dynamicRoutes['GET /sites/:siteId/llmo/sheet-data/:sheetType/:week/:dataSource'].paramNames).to.deep.equal(['siteId', 'sheetType', 'week', 'dataSource']); expect(dynamicRoutes['GET /sites/:siteId/llmo/config'].handler).to.equal(mockLlmoController.getLlmoConfig); expect(dynamicRoutes['GET /sites/:siteId/llmo/config'].paramNames).to.deep.equal(['siteId']); - expect(dynamicRoutes['GET /sites/:siteId/llmo/questions'].handler).to.equal(mockLlmoController.getLlmoQuestions); - expect(dynamicRoutes['GET /sites/:siteId/llmo/questions'].paramNames).to.deep.equal(['siteId']); - expect(dynamicRoutes['POST /sites/:siteId/llmo/questions'].handler).to.equal(mockLlmoController.addLlmoQuestion); - expect(dynamicRoutes['POST /sites/:siteId/llmo/questions'].paramNames).to.deep.equal(['siteId']); - expect(dynamicRoutes['DELETE /sites/:siteId/llmo/questions/:questionKey'].handler).to.equal(mockLlmoController.removeLlmoQuestion); - expect(dynamicRoutes['DELETE /sites/:siteId/llmo/questions/:questionKey'].paramNames).to.deep.equal(['siteId', 'questionKey']); - expect(dynamicRoutes['PATCH /sites/:siteId/llmo/questions/:questionKey'].handler).to.equal(mockLlmoController.patchLlmoQuestion); - expect(dynamicRoutes['PATCH /sites/:siteId/llmo/questions/:questionKey'].paramNames).to.deep.equal(['siteId', 'questionKey']); expect(dynamicRoutes['GET /sites/:siteId/llmo/customer-intent'].handler).to.equal(mockLlmoController.getLlmoCustomerIntent); expect(dynamicRoutes['GET /sites/:siteId/llmo/customer-intent'].paramNames).to.deep.equal(['siteId']); expect(dynamicRoutes['POST /sites/:siteId/llmo/customer-intent'].handler).to.equal(mockLlmoController.addLlmoCustomerIntent);