From 88bb49105a9b0a9cd297fe9667aff1df24bcee03 Mon Sep 17 00:00:00 2001 From: Bruce Lefebvre Date: Tue, 18 Jun 2024 16:03:11 -0400 Subject: [PATCH] fix: #268 Import URL entities need a REDIRECT status, and additional properties for reporting (#269) In order to support generating the import-report.xlsx file for an import job, we need the ability to track if a URL was skipped because it was a redirect, or if it failed for another reason. --- package-lock.json | 10 +-- .../src/dto/import-url.js | 6 ++ .../src/models/importer/import-url.js | 55 ++++++++++++- .../unit/models/importer/import-url.test.js | 80 ++++++++++++++++++- .../unit/service/import-url/index.test.js | 2 +- 5 files changed, 140 insertions(+), 13 deletions(-) diff --git a/package-lock.json b/package-lock.json index 19ea273d..64a0277e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -22086,7 +22086,7 @@ }, "packages/spacecat-shared-data-access": { "name": "@adobe/spacecat-shared-data-access", - "version": "1.29.0", + "version": "1.29.1", "license": "Apache-2.0", "dependencies": { "@adobe/spacecat-shared-dynamo": "1.2.5", @@ -22601,7 +22601,7 @@ }, "packages/spacecat-shared-dynamo": { "name": "@adobe/spacecat-shared-dynamo", - "version": "1.3.23", + "version": "1.3.24", "license": "Apache-2.0", "dependencies": { "@adobe/spacecat-shared-utils": "1.1.0", @@ -22630,7 +22630,7 @@ }, "packages/spacecat-shared-google-client": { "name": "@adobe/spacecat-shared-google-client", - "version": "1.1.9", + "version": "1.1.10", "license": "Apache-2.0", "dependencies": { "@adobe/fetch": "4.1.8", @@ -24313,7 +24313,7 @@ }, "packages/spacecat-shared-slack-client": { "name": "@adobe/spacecat-shared-slack-client", - "version": "1.3.6", + "version": "1.3.7", "license": "Apache-2.0", "dependencies": { "@adobe/helix-universal": "5.0.5", @@ -24360,7 +24360,7 @@ }, "packages/spacecat-shared-utils": { "name": "@adobe/spacecat-shared-utils", - "version": "1.15.8", + "version": "1.15.10", "license": "Apache-2.0", "dependencies": { "@adobe/fetch": "4.1.8", diff --git a/packages/spacecat-shared-data-access/src/dto/import-url.js b/packages/spacecat-shared-data-access/src/dto/import-url.js index c00eae9b..3e45ad9d 100644 --- a/packages/spacecat-shared-data-access/src/dto/import-url.js +++ b/packages/spacecat-shared-data-access/src/dto/import-url.js @@ -26,6 +26,9 @@ export const ImportUrlDto = { jobId: importUrl.getJobId(), url: importUrl.getUrl(), status: importUrl.getStatus(), + reason: importUrl.getReason(), + path: importUrl.getPath(), + file: importUrl.getFile(), }), /** @@ -37,6 +40,9 @@ export const ImportUrlDto = { jobId: dynamoItem.jobId, url: dynamoItem.url, status: dynamoItem.status, + reason: dynamoItem.reason, + path: dynamoItem.path, + file: dynamoItem.file, }; return createImportUrl(importUrlData); }, diff --git a/packages/spacecat-shared-data-access/src/models/importer/import-url.js b/packages/spacecat-shared-data-access/src/models/importer/import-url.js index 34c65e53..d44edf3b 100644 --- a/packages/spacecat-shared-data-access/src/models/importer/import-url.js +++ b/packages/spacecat-shared-data-access/src/models/importer/import-url.js @@ -10,12 +10,13 @@ * governing permissions and limitations under the License. */ -import { isValidUrl } from '@adobe/spacecat-shared-utils'; +import { hasText, isValidUrl } from '@adobe/spacecat-shared-utils'; import { Base } from '../base.js'; import { ImportJobStatus } from './import-job.js'; export const ImportUrlStatus = { PENDING: 'PENDING', + REDIRECT: 'REDIRECT', ...ImportJobStatus, }; @@ -31,11 +32,16 @@ const ImportUrl = (data) => { self.getJobId = () => self.state.jobId; self.getUrl = () => self.state.url; self.getStatus = () => self.state.status; + self.getReason = () => self.state.reason; + // Absolute path to the resource that is being imported for the given URL + self.getPath = () => self.state.path; + // Resulting path and filename of the imported .docx file + self.getFile = () => self.state.file; /** - * Updates the status of the ImportUrl - */ - self.updateStatus = (status) => { + * Updates the status of the ImportUrl + */ + self.setStatus = (status) => { if (!Object.values(ImportUrlStatus).includes(status)) { throw new Error(`Invalid Import URL status during update: ${status}`); } @@ -45,6 +51,47 @@ const ImportUrl = (data) => { return self; }; + + /** + * Updates the reason that the import of this URL was not successful + */ + self.setReason = (reason) => { + if (!hasText(reason)) { + return self; // no-op + } + + self.state.reason = reason; + self.touch(); + return self; + }; + + /** + * Updates the path of the ImportUrl + */ + self.setPath = (path) => { + if (!hasText(path)) { + return self; // no-op + } + + self.state.path = path; + self.touch(); + return self; + }; + + /** + * Updates the file of the ImportUrl. This is the path and file name of the file which + * was imported. + */ + self.setFile = (file) => { + if (!hasText(file)) { + return self; // no-op + } + + self.state.file = file; + self.touch(); + return self; + }; + return Object.freeze(self); }; diff --git a/packages/spacecat-shared-data-access/test/unit/models/importer/import-url.test.js b/packages/spacecat-shared-data-access/test/unit/models/importer/import-url.test.js index 3440e71d..a818ca9a 100644 --- a/packages/spacecat-shared-data-access/test/unit/models/importer/import-url.test.js +++ b/packages/spacecat-shared-data-access/test/unit/models/importer/import-url.test.js @@ -13,7 +13,8 @@ /* eslint-env mocha */ import { expect } from 'chai'; -import { createImportUrl } from '../../../../src/models/importer/import-url.js'; +import { createImportUrl, ImportUrlStatus } from '../../../../src/models/importer/import-url.js'; +import { ImportUrlDto } from '../../../../src/dto/import-url.js'; const validImportUrlData = { id: '123', @@ -22,6 +23,16 @@ const validImportUrlData = { status: 'RUNNING', }; +const importUrlRedirectData = { + id: '456', + url: 'https://www.example.com/redirect', + jobId: '456', + status: 'REDIRECT', + reason: 'https://www.example.com/redirect/destination', + path: '/test-data', + file: '/test-data.docx', +}; + describe('ImportUrl Model tests', () => { describe('Validation Tests', () => { it('throws an error if url is not a valid URL', () => { @@ -34,21 +45,84 @@ describe('ImportUrl Model tests', () => { }); describe('Import URL Functionality Tests', () => { let importUrl; + beforeEach(() => { importUrl = createImportUrl({ ...validImportUrlData }); }); + it('updates the status of the import URL', () => { - importUrl.updateStatus('COMPLETE'); + importUrl.setStatus('COMPLETE'); expect(importUrl.getStatus()).to.equal('COMPLETE'); }); + it('returns the url attribute of the import URL', () => { expect(importUrl.getUrl()).to.equal('https://www.example.com'); }); + it('returns the job ID of the import URL', () => { expect(importUrl.getJobId()).to.equal('456'); }); + it('throws an error if the status is invalid', () => { - expect(() => importUrl.updateStatus('invalid')).to.throw('Invalid Import URL status during update: invalid'); + expect(() => importUrl.setStatus('invalid')).to.throw('Invalid Import URL status during update: invalid'); + }); + + it('updates the status and reason for a url', () => { + importUrl.setStatus('REDIRECT'); + importUrl.setReason('https://www.example.com/redirect/destination'); + expect(importUrl.getStatus()).to.equal(ImportUrlStatus.REDIRECT); + expect(importUrl.getReason()).to.equal('https://www.example.com/redirect/destination'); + }); + + it('does not update properties if the setters are passed invalid data', () => { + const importUrlRedirect = createImportUrl(importUrlRedirectData); + + importUrlRedirect.setReason(undefined); + expect(importUrlRedirect.getReason()).to.equal('https://www.example.com/redirect/destination'); + + importUrlRedirect.setPath(null); + expect(importUrlRedirect.getPath()).to.equal('/test-data'); + + importUrlRedirect.setFile(''); + expect(importUrlRedirect.getFile()).to.equal('/test-data.docx'); + }); + + it('updates the file and path for a url', () => { + importUrl.setStatus('COMPLETE'); + importUrl.setPath('/index'); + importUrl.setFile('/index.docx'); + + expect(importUrl.getStatus()).to.equal(ImportUrlStatus.COMPLETE); + expect(importUrl.getPath()).to.equal('/index'); + expect(importUrl.getFile()).to.equal('/index.docx'); + }); + }); + + describe('Import URL DTO Tests', () => { + it('should serialize to a Dynamo-compatible object', () => { + const importUrlRedirect = createImportUrl(importUrlRedirectData); + expect(ImportUrlDto.toDynamoItem(importUrlRedirect)).to.deep.equal({ + id: '456', + url: 'https://www.example.com/redirect', + jobId: '456', + status: 'REDIRECT', + reason: 'https://www.example.com/redirect/destination', + path: '/test-data', + file: '/test-data.docx', + }); + }); + + it('should deserialize from a Dynamo object', () => { + const urlFromDynamo = ImportUrlDto.fromDynamoItem(importUrlRedirectData); + + const importUrlRedirect = createImportUrl(importUrlRedirectData); + expect(urlFromDynamo.getId()).to.deep.equal(importUrlRedirect.getId()); + expect(urlFromDynamo.getUrl()).to.deep.equal(importUrlRedirect.getUrl()); + expect(urlFromDynamo.getJobId()).to.deep.equal(importUrlRedirect.getJobId()); + expect(urlFromDynamo.getStatus()).to.deep.equal(importUrlRedirect.getStatus()); + expect(urlFromDynamo.getReason()).to.deep.equal(importUrlRedirect.getReason()); + expect(urlFromDynamo.getPath()).to.deep.equal(importUrlRedirect.getPath()); + expect(urlFromDynamo.getFile()).to.deep.equal(importUrlRedirect.getFile()); }); }); }); diff --git a/packages/spacecat-shared-data-access/test/unit/service/import-url/index.test.js b/packages/spacecat-shared-data-access/test/unit/service/import-url/index.test.js index 85a7e491..8702d318 100644 --- a/packages/spacecat-shared-data-access/test/unit/service/import-url/index.test.js +++ b/packages/spacecat-shared-data-access/test/unit/service/import-url/index.test.js @@ -88,7 +88,7 @@ describe('Import Url Tests', () => { mockDynamoClient.getItem.resolves(mockImportUrl); const importUrl = await exportedFunctions.getImportUrlById('test-id'); - importUrl.updateStatus('COMPLETE'); + importUrl.setStatus('COMPLETE'); const result = await exportedFunctions.updateImportUrl(importUrl); expect(result).to.be.not.null;