From 78ce9a39def7eb34e78dc24eacb64de50c2de744 Mon Sep 17 00:00:00 2001 From: Damian Zehnder <16799758+dzehnder@users.noreply.github.com> Date: Fri, 29 Nov 2024 14:04:14 +0100 Subject: [PATCH] feat: introduce Opportunity creation for Broken Backlinks (#475) --- README.md | 71 ++++++++ src/backlinks/handler.js | 72 +++++++- src/utils/data-access.js | 67 ++++++++ test/audits/backlinks.test.js | 295 +++++++++++++++++++++++++++++++-- test/utils/data-access.test.js | 189 +++++++++++++++++---- 5 files changed, 646 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index 23d5a777..2c11bcb6 100644 --- a/README.md +++ b/README.md @@ -216,3 +216,74 @@ export default new AuditBuilder() .build(); ``` + +### How to add Opportunities and Suggestions +```js +import { syncSuggestions } from '../utils/data-access.js'; + +export async function auditRunner(url, context) { + + // your audit logic goes here... + + return { + auditResult: results, + fullAuditRef: baseURL, + }; +} + +async function convertToOpportunity(auditUrl, auditData, context) { + const { dataAccess } = context; + + const opportunities = await dataAccess.Opportunity.allBySiteIdAndStatus(auditData.siteId, 'NEW'); + const opportunity = opportunities.find((oppty) => oppty.getType() === 'audit-type'); + + if (!opportunity) { + const opportunityData = { + siteId: auditData.siteId, + auditId: auditData.id, + runbook: 'link-to-runbook', + type: 'audit-type', + origin: 'AUTOMATON', + title: 'Opportunity Title', + description: 'Opportunity Description', + guidance: { + steps: [ + 'Step 1', + 'Step 2', + ], + }, + tags: ['tag1', 'tag2'], + }; + opportunity = await dataAccess.Opportunity.create(opportunityData); + } else { + opportunity.setAuditId(auditData.id); + await opportunity.save(); + } + + // this logic changes based on the audit type + const buildKey = (auditData) => `${auditData.property}|${auditData.anotherProperty}`; + + await syncSuggestions({ + opportunity, + newData: auditData, + buildKey, + mapNewSuggestion: (issue) => ({ + opportunityId: opportunity.getId(), + type: 'SUGGESTION_TYPE', + rank: issue.rankMetric, + // data changes based on the audit type + data: { + property: issue.property, + anotherProperty: issue.anotherProperty + } + }), + log + }); +} + +export default new AuditBuilder() + .withRunner(auditRunner) + .withPostProcessors([ convertToOpportunity ]) + .build(); + +``` diff --git a/src/backlinks/handler.js b/src/backlinks/handler.js index 39126a09..4bfad765 100644 --- a/src/backlinks/handler.js +++ b/src/backlinks/handler.js @@ -16,7 +16,7 @@ import { import { composeAuditURL, tracingFetch as fetch } from '@adobe/spacecat-shared-utils'; import AhrefsAPIClient from '@adobe/spacecat-shared-ahrefs-client'; import { AbortController, AbortError } from '@adobe/fetch'; -import { retrieveSiteBySiteId } from '../utils/data-access.js'; +import { retrieveSiteBySiteId, syncSuggestions } from '../utils/data-access.js'; import { enhanceBacklinksWithFixes } from '../support/utils.js'; const TIMEOUT = 3000; @@ -139,14 +139,78 @@ export default async function auditBrokenBacklinks(message, context) { auditResult, }; - await dataAccess.addAudit(auditData); - const data = { + const audit = await dataAccess.addAudit(auditData); + const result = { type, url: site.getBaseURL(), auditContext, auditResult, }; - await sqs.sendMessage(queueUrl, data); + + let brokenBacklinksOppty; + + try { + const opportunities = await dataAccess.Opportunity.allBySiteIdAndStatus(siteId, 'NEW'); + brokenBacklinksOppty = opportunities.find((oppty) => oppty.getType() === 'broken-backlinks'); + } catch (e) { + log.error(`Fetching opportunities for siteId ${siteId} failed with error: ${e.message}`); + return internalServerError(`Failed to fetch opportunities for siteId ${siteId}: ${e.message}`); + } + + try { + if (!brokenBacklinksOppty) { + const opportunityData = { + siteId: site.getId(), + auditId: audit.getId(), + runbook: 'https://adobe.sharepoint.com/:w:/r/sites/aemsites-engineering/_layouts/15/doc2.aspx?sourcedoc=%7BAC174971-BA97-44A9-9560-90BE6C7CF789%7D&file=Experience_Success_Studio_Broken_Backlinks_Runbook.docx&action=default&mobileredirect=true', + type: 'broken-backlinks', + origin: 'AUTOMATION', + title: 'Authoritative Domains are linking to invalid URLs. This could impact your SEO.', + description: 'Provide the correct target URL that each of the broken backlinks should be redirected to.', + guidance: { + steps: [ + 'Review the list of broken target URLs and the suggested redirects.', + 'Manually override redirect URLs as needed.', + 'Copy redirects.', + 'Paste new entries in your website redirects file.', + 'Publish the changes.', + ], + }, + tags: ['Traffic acquisition'], + }; + + brokenBacklinksOppty = await dataAccess.Opportunity.create(opportunityData); + } else { + brokenBacklinksOppty.setAuditId(audit.getId()); + await brokenBacklinksOppty.save(); + } + } catch (e) { + log.error(`Creating opportunity for siteId ${siteId} failed with error: ${e.message}`, e); + return internalServerError(`Failed to create opportunity for siteId ${siteId}: ${e.message}`); + } + + if (!result.auditResult.error) { + const buildKey = (data) => `${data.url_from}|${data.url_to}`; + + await syncSuggestions({ + opportunity: brokenBacklinksOppty, + newData: result.auditResult.brokenBacklinks, + buildKey, + mapNewSuggestion: (backlink) => ({ + opportunityId: brokenBacklinksOppty.getId(), + type: 'REDIRECT_UPDATE', + rank: backlink.traffic_domain, + data: { + title: backlink.title, + url_from: backlink.url_from, + url_to: backlink.url_to, + traffic_domain: backlink.traffic_domain, + }, + }), + log, + }); + } + await sqs.sendMessage(queueUrl, result); log.info(`Successfully audited ${siteId} for ${type} type audit`); return noContent(); diff --git a/src/utils/data-access.js b/src/utils/data-access.js index bd3e085e..d7b734a8 100644 --- a/src/utils/data-access.js +++ b/src/utils/data-access.js @@ -35,3 +35,70 @@ export async function retrieveSiteBySiteId(dataAccess, siteId, log) { throw new Error(`Error getting site ${siteId}: ${e.message}`); } } + +/** + * Synchronizes existing suggestions with new data by removing outdated suggestions + * and adding new ones. + * + * @param {Object} params - The parameters for the sync operation. + * @param {Object} params.opportunity - The opportunity object to synchronize suggestions for. + * @param {Array} params.newData - Array of new data objects to sync. + * @param {Function} params.buildKey - Function to generate a unique key for each item. + * @param {Function} params.mapNewSuggestion - Function to map new data to suggestion objects. + * @param {Object} params.log - Logger object for error reporting. + * @returns {Promise} - Resolves when the synchronization is complete. + */ +export async function syncSuggestions({ + opportunity, + newData, + buildKey, + mapNewSuggestion, + log, +}) { + const newDataKeys = new Set(newData.map(buildKey)); + const existingSuggestions = await opportunity.getSuggestions(); + + // Remove outdated suggestions + await Promise.all( + existingSuggestions + .filter((existing) => !newDataKeys.has(buildKey(existing))) + .map((suggestion) => suggestion.remove()), + ); + + // Update existing suggestions + await Promise.all( + existingSuggestions + .filter((existing) => { + const existingKey = buildKey(existing); + return newDataKeys.has(existingKey); + }) + .map((existing) => { + const newDataItem = newData.find((data) => buildKey(data) === buildKey(existing)); + existing.setData(newDataItem); + return existing.save(); + }), + ); + + // Prepare new suggestions + const newSuggestions = newData + .filter((data) => !existingSuggestions.some( + (existing) => buildKey(existing) === buildKey(data), + )) + .map(mapNewSuggestion); + + // Add new suggestions if any + if (newSuggestions.length > 0) { + const suggestions = await opportunity.addSuggestions(newSuggestions); + + if (suggestions.errorItems?.length > 0) { + log.error(`Suggestions for siteId ${opportunity.getSiteId()} contains ${suggestions.errorItems.length} items with errors`); + suggestions.errorItems.forEach((errorItem) => { + log.error(`Item ${JSON.stringify(errorItem.item)} failed with error: ${errorItem.error}`); + }); + + if (suggestions.createdItems?.length <= 0) { + throw new Error(`Failed to create suggestions for siteId ${opportunity.getSiteId()}`); + } + } + } +} diff --git a/test/audits/backlinks.test.js b/test/audits/backlinks.test.js index e310ed56..e62a4e2f 100644 --- a/test/audits/backlinks.test.js +++ b/test/audits/backlinks.test.js @@ -109,7 +109,7 @@ describe('Backlinks Tests', function () { title: 'backlink that times out', url_from: 'https://from.com/from-4', url_to: 'https://foo.com/times-out', - domain_traffic: 500, + traffic_domain: 500, }; const auditResult = { @@ -118,19 +118,19 @@ describe('Backlinks Tests', function () { title: 'backlink that returns 404', url_from: 'https://from.com/from-1', url_to: 'https://foo.com/returns-404', - domain_traffic: 4000, + traffic_domain: 4000, }, { title: 'backlink that redirects to www and throw connection error', url_from: 'https://from.com/from-2', url_to: 'https://foo.com/redirects-throws-error', - domain_traffic: 2000, + traffic_domain: 2000, }, { title: 'backlink that returns 429', url_from: 'https://from.com/from-3', url_to: 'https://foo.com/returns-429', - domain_traffic: 1000, + traffic_domain: 1000, }, ], }; @@ -171,20 +171,20 @@ describe('Backlinks Tests', function () { title: 'backlink that redirects to www and throw connection error', url_from: 'https://from.com/from-2', url_to: 'https://foo.com/redirects-throws-error', - domain_traffic: 2000, + traffic_domain: 2000, }, { title: 'backlink that returns 429', url_from: 'https://from.com/from-3', url_to: 'https://foo.com/returns-429', - domain_traffic: 1000, + traffic_domain: 1000, url_suggested: 'https://bar.foo.com/bar.html', }, { title: 'backlink that is not excluded', url_from: 'https://from.com/from-not-excluded', url_to: 'https://foo.com/not-excluded', - domain_traffic: 5000, + traffic_domain: 5000, }, ], fullAuditRef: sinon.match.string, @@ -195,16 +195,63 @@ describe('Backlinks Tests', function () { title: 'backlink that is not excluded', url_from: 'https://from.com/from-not-excluded', url_to: 'https://foo.com/not-excluded', - domain_traffic: 5000, + traffic_domain: 5000, }, ]; + const audit = { + getId: () => 'test-audit-id', + }; + + let brokenBacklinksOpportunity; + let brokenBacklinksSuggestions; + let brokenBacklinkExistingSuggestions; + let otherOpportunity; + beforeEach(() => { + brokenBacklinksOpportunity = { + getType: () => 'broken-backlinks', + getId: () => 'test-opportunity-id', + getSiteId: () => site.getId(), + addSuggestions: sinon.stub(), + getSuggestions: sinon.stub(), + setAuditId: sinon.stub(), + save: sinon.stub().resolves(), + }; + + brokenBacklinksSuggestions = { + createdItems: auditResult.backlinks, + errorItems: [], + }; + + brokenBacklinkExistingSuggestions = [{ + opportunityId: brokenBacklinksOpportunity.getId(), + type: 'REDIRECT_UPDATE', + rank: 5000, + data: { + title: 'backlink that is not excluded', + url_from: 'https://from.com/from-not-excluded', + url_to: 'https://foo.com/not-excluded', + traffic_domain: 5000, + }, + remove: sinon.stub(), + }]; + + brokenBacklinkExistingSuggestions[0].remove.resolves(); + + otherOpportunity = { + getType: () => 'other', + }; + mockDataAccess = { getSiteByID: sinon.stub(), - addAudit: sinon.stub(), + addAudit: sinon.stub().resolves(audit), getTopPagesForSite: sinon.stub(), getConfiguration: sinon.stub(), + Opportunity: { + allBySiteIdAndStatus: sinon.stub(), + create: sinon.stub(), + }, }; message = { @@ -261,6 +308,12 @@ describe('Backlinks Tests', function () { mockDataAccess.getTopPagesForSite.resolves([siteTopPage, siteTopPage2]); mockDataAccess.getSiteByID = sinon.stub().withArgs('site1').resolves(siteWithExcludedUrls); mockDataAccess.getConfiguration = sinon.stub().resolves(configuration); + brokenBacklinksOpportunity.setAuditId.returns(brokenBacklinksOpportunity); + brokenBacklinksOpportunity.addSuggestions.resolves(brokenBacklinksSuggestions); + brokenBacklinksOpportunity.getSuggestions.resolves(brokenBacklinkExistingSuggestions); + mockDataAccess.Opportunity.allBySiteIdAndStatus.resolves( + [otherOpportunity, brokenBacklinksOpportunity], + ); nock(siteWithExcludedUrls.getBaseURL()) .get(/.*/) @@ -290,10 +343,16 @@ describe('Backlinks Tests', function () { ); }); - it('should successfully perform an audit to detect broken backlinks, save and send the proper audit result', async () => { + it('should successfully perform an audit to detect broken backlinks, save and add the proper audit result to an existing opportunity', async () => { mockDataAccess.getSiteByID = sinon.stub().withArgs('site1').resolves(site); mockDataAccess.getTopPagesForSite.resolves([]); mockDataAccess.getConfiguration = sinon.stub().resolves(configuration); + brokenBacklinksOpportunity.setAuditId.returns(brokenBacklinksOpportunity); + brokenBacklinksOpportunity.addSuggestions.resolves(brokenBacklinksSuggestions); + brokenBacklinksOpportunity.getSuggestions.resolves([]); + mockDataAccess.Opportunity.allBySiteIdAndStatus.resolves( + [otherOpportunity, brokenBacklinksOpportunity], + ); nock(site.getBaseURL()) .get(/.*/) @@ -320,16 +379,119 @@ describe('Backlinks Tests', function () { expect(response.status).to.equal(204); expect(mockDataAccess.addAudit).to.have.been.calledOnce; + expect(brokenBacklinksOpportunity.setAuditId).to.have.been.calledOnce; + expect(brokenBacklinksOpportunity.addSuggestions).to.have.been.calledOnce; expect(context.sqs.sendMessage).to.have.been.calledOnce; expect(context.sqs.sendMessage).to.have.been .calledWith(context.env.AUDIT_RESULTS_QUEUE_URL, expectedMessage); expect(context.log.info).to.have.been.calledWith('Successfully audited site1 for broken-backlinks type audit'); }); + it('should successfully perform an audit to detect broken backlinks, save and add the proper audit result to a new opportunity', async () => { + mockDataAccess.getSiteByID = sinon.stub().withArgs('site1').resolves(site); + mockDataAccess.getTopPagesForSite.resolves([]); + mockDataAccess.getConfiguration = sinon.stub().resolves(configuration); + mockDataAccess.Opportunity.create.resolves(brokenBacklinksOpportunity); + brokenBacklinksOpportunity.addSuggestions.resolves(brokenBacklinksSuggestions); + brokenBacklinksOpportunity.getSuggestions.resolves([]); + mockDataAccess.Opportunity.allBySiteIdAndStatus.resolves([otherOpportunity]); + + nock(site.getBaseURL()) + .get(/.*/) + .reply(200); + + nock('https://ahrefs.com') + .get(/.*/) + .reply(200, auditResult); + + const expectedMessage = { + type: message.type, + url: site.getBaseURL(), + auditContext: { + finalUrl: 'bar.foo.com', + }, + auditResult: { + finalUrl: 'bar.foo.com', + brokenBacklinks: auditResult.backlinks, + fullAuditRef: 'https://ahrefs.com/site-explorer/broken-backlinks?select=title%2Curl_from%2Curl_to%2Ctraffic_domain&limit=50&mode=prefix&order_by=domain_rating_source%3Adesc%2Ctraffic_domain%3Adesc&target=bar.foo.com&output=json&where=%7B%22and%22%3A%5B%7B%22field%22%3A%22domain_rating_source%22%2C%22is%22%3A%5B%22gte%22%2C29.5%5D%7D%2C%7B%22field%22%3A%22traffic_domain%22%2C%22is%22%3A%5B%22gte%22%2C500%5D%7D%2C%7B%22field%22%3A%22links_external%22%2C%22is%22%3A%5B%22lte%22%2C300%5D%7D%5D%7D', + }, + }; + + const response = await auditBrokenBacklinks(message, context); + + expect(response.status).to.equal(204); + expect(mockDataAccess.addAudit).to.have.been.calledOnce; + expect(mockDataAccess.Opportunity.create).to.have.been.calledOnce; + expect(brokenBacklinksOpportunity.addSuggestions).to.have.been.calledOnce; + expect(context.sqs.sendMessage).to.have.been.calledOnce; + expect(context.sqs.sendMessage).to.have.been + .calledWith(context.env.AUDIT_RESULTS_QUEUE_URL, expectedMessage); + expect(context.log.info).to.have.been.calledWith('Successfully audited site1 for broken-backlinks type audit'); + }); + + it('should perform a partial successful audit to detect broken backlinks, save and add the proper audit result to an existing opportunity', async () => { + mockDataAccess.getSiteByID = sinon.stub().withArgs('site1').resolves(site); + mockDataAccess.getTopPagesForSite.resolves([]); + mockDataAccess.getConfiguration = sinon.stub().resolves(configuration); + brokenBacklinksOpportunity.setAuditId.returns(brokenBacklinksOpportunity); + brokenBacklinksOpportunity.addSuggestions.resolves({ + createdItems: auditResult.backlinks, + errorItems: [{ + item: auditResult.backlinks[0], + error: 'ValidationError', + }], + }); + brokenBacklinksOpportunity.getSuggestions.resolves([]); + mockDataAccess.Opportunity.allBySiteIdAndStatus.resolves( + [otherOpportunity, brokenBacklinksOpportunity], + ); + + nock(site.getBaseURL()) + .get(/.*/) + .reply(200); + + nock('https://ahrefs.com') + .get(/.*/) + .reply(200, auditResult); + + const expectedMessage = { + type: message.type, + url: site.getBaseURL(), + auditContext: { + finalUrl: 'bar.foo.com', + }, + auditResult: { + finalUrl: 'bar.foo.com', + brokenBacklinks: auditResult.backlinks, + fullAuditRef: 'https://ahrefs.com/site-explorer/broken-backlinks?select=title%2Curl_from%2Curl_to%2Ctraffic_domain&limit=50&mode=prefix&order_by=domain_rating_source%3Adesc%2Ctraffic_domain%3Adesc&target=bar.foo.com&output=json&where=%7B%22and%22%3A%5B%7B%22field%22%3A%22domain_rating_source%22%2C%22is%22%3A%5B%22gte%22%2C29.5%5D%7D%2C%7B%22field%22%3A%22traffic_domain%22%2C%22is%22%3A%5B%22gte%22%2C500%5D%7D%2C%7B%22field%22%3A%22links_external%22%2C%22is%22%3A%5B%22lte%22%2C300%5D%7D%5D%7D', + }, + }; + + const response = await auditBrokenBacklinks(message, context); + + expect(response.status).to.equal(204); + expect(mockDataAccess.addAudit).to.have.been.calledOnce; + expect(brokenBacklinksOpportunity.setAuditId).to.have.been.calledOnce; + expect(brokenBacklinksOpportunity.addSuggestions).to.have.been.calledOnce; + expect(context.sqs.sendMessage).to.have.been.calledOnce; + expect(context.sqs.sendMessage).to.have.been + .calledWith(context.env.AUDIT_RESULTS_QUEUE_URL, expectedMessage); + expect(context.log.info).to.have.been.calledWith('Successfully audited site1 for broken-backlinks type audit'); + expect(context.log.error).to.have.been.calledWith( + 'Suggestions for siteId site1 contains 1 items with errors', + ); + }); + it('should successfully perform an audit to detect broken backlinks, save and send the proper audit result for message containing siteId', async () => { mockDataAccess.getSiteByID = sinon.stub().withArgs('site1').resolves(site); mockDataAccess.getTopPagesForSite.resolves([]); mockDataAccess.getConfiguration = sinon.stub().resolves(configuration); + brokenBacklinksOpportunity.setAuditId.returns(brokenBacklinksOpportunity); + brokenBacklinksOpportunity.addSuggestions.resolves(brokenBacklinksSuggestions); + brokenBacklinksOpportunity.getSuggestions.resolves([]); + mockDataAccess.Opportunity.allBySiteIdAndStatus.resolves( + [otherOpportunity, brokenBacklinksOpportunity], + ); message.siteId = message.url; delete message.url; @@ -371,6 +533,12 @@ describe('Backlinks Tests', function () { mockDataAccess.getSiteByID = sinon.stub().withArgs('site1').resolves(site); mockDataAccess.getTopPagesForSite.resolves([siteTopPage, siteTopPage2]); mockDataAccess.getConfiguration = sinon.stub().resolves(configuration); + brokenBacklinksOpportunity.setAuditId.returns(brokenBacklinksOpportunity); + brokenBacklinksOpportunity.addSuggestions.resolves(brokenBacklinksSuggestions); + brokenBacklinksOpportunity.getSuggestions.resolves([]); + mockDataAccess.Opportunity.allBySiteIdAndStatus.resolves( + [otherOpportunity, brokenBacklinksOpportunity], + ); nock(site.getBaseURL()) .get(/.*/) @@ -410,6 +578,12 @@ describe('Backlinks Tests', function () { mockDataAccess.getSiteByID = sinon.stub().withArgs('site2').resolves(site2); configuration.disableHandlerForSite('broken-backlinks-auto-suggest', { getId: () => site2.getId(), getOrganizationId: () => org.getId() }); mockDataAccess.getConfiguration = sinon.stub().resolves(configuration); + brokenBacklinksOpportunity.setAuditId.returns(brokenBacklinksOpportunity); + brokenBacklinksOpportunity.addSuggestions.resolves(brokenBacklinksSuggestions); + brokenBacklinksOpportunity.getSuggestions.resolves([]); + mockDataAccess.Opportunity.allBySiteIdAndStatus.resolves( + [otherOpportunity, brokenBacklinksOpportunity], + ); nock(site2.getBaseURL()) .get(/.*/) @@ -470,6 +644,12 @@ describe('Backlinks Tests', function () { }], }; mockDataAccess.getConfiguration = sinon.stub().resolves(configuration); + brokenBacklinksOpportunity.setAuditId.returns(brokenBacklinksOpportunity); + brokenBacklinksOpportunity.addSuggestions.resolves(brokenBacklinksSuggestions); + brokenBacklinksOpportunity.getSuggestions.resolves([]); + mockDataAccess.Opportunity.allBySiteIdAndStatus.resolves( + [otherOpportunity, brokenBacklinksOpportunity], + ); nock(site.getBaseURL()) .get(/.*/) .reply(200); @@ -503,6 +683,12 @@ describe('Backlinks Tests', function () { mockDataAccess.getSiteByID = sinon.stub().withArgs('site2').resolves(site2); mockDataAccess.getTopPagesForSite.resolves([]); mockDataAccess.getConfiguration = sinon.stub().resolves(configuration); + brokenBacklinksOpportunity.setAuditId.returns(brokenBacklinksOpportunity); + brokenBacklinksOpportunity.addSuggestions.resolves(brokenBacklinksSuggestions); + brokenBacklinksOpportunity.getSuggestions.resolves([]); + mockDataAccess.Opportunity.allBySiteIdAndStatus.resolves( + [otherOpportunity, brokenBacklinksOpportunity], + ); nock(site2.getBaseURL()) .get(/.*/) @@ -545,6 +731,12 @@ describe('Backlinks Tests', function () { mockDataAccess.getSiteByID = sinon.stub().withArgs('site2').resolves(site2); mockDataAccess.getTopPagesForSite.resolves([]); mockDataAccess.getConfiguration = sinon.stub().resolves(configuration); + brokenBacklinksOpportunity.setAuditId.returns(brokenBacklinksOpportunity); + brokenBacklinksOpportunity.addSuggestions.resolves(brokenBacklinksSuggestions); + brokenBacklinksOpportunity.getSuggestions.resolves([]); + mockDataAccess.Opportunity.allBySiteIdAndStatus.resolves( + [otherOpportunity, brokenBacklinksOpportunity], + ); const fixedBacklinks = [ { @@ -647,6 +839,84 @@ describe('Backlinks Tests', function () { expect(mockLog.error).to.have.been.calledTwice; }); + it('should return a 500 if suggestions cannot be added to an opportunity', async () => { + mockDataAccess.getSiteByID = sinon.stub().withArgs('site1').resolves(site); + mockDataAccess.getTopPagesForSite.resolves([]); + mockDataAccess.getConfiguration = sinon.stub().resolves(configuration); + brokenBacklinksOpportunity.setAuditId.returns(brokenBacklinksOpportunity); + brokenBacklinksOpportunity.addSuggestions.resolves({ + createdItems: [], + errorItems: [{ + item: auditResult.backlinks[0], + error: 'ValidationError', + }], + }); + brokenBacklinksOpportunity.getSuggestions.resolves([]); + mockDataAccess.Opportunity.allBySiteIdAndStatus.resolves( + [otherOpportunity, brokenBacklinksOpportunity], + ); + + nock(site.getBaseURL()) + .get(/.*/) + .reply(200); + + nock('https://ahrefs.com') + .get(/.*/) + .reply(200, auditResult); + + const response = await auditBrokenBacklinks(message, context); + + expect(response.status).to.equal(500); + expect(mockDataAccess.addAudit).to.have.been.calledOnce; + expect(brokenBacklinksOpportunity.setAuditId).to.have.been.calledOnce; + expect(brokenBacklinksOpportunity.addSuggestions).to.have.been.calledOnce; + expect(context.log.error).to.have.been.calledWith('Suggestions for siteId site1 contains 1 items with errors'); + }); + + it('should return 500 when opportunity fetching fails', async () => { + mockDataAccess.getSiteByID = sinon.stub().withArgs('site1').resolves(site); + mockDataAccess.getTopPagesForSite.resolves([]); + mockDataAccess.getConfiguration = sinon.stub().resolves(configuration); + mockDataAccess.Opportunity.allBySiteIdAndStatus.rejects(new Error('oppty error')); + + nock(site.getBaseURL()) + .get(/.*/) + .reply(200); + + nock('https://ahrefs.com') + .get(/.*/) + .reply(200, auditResult); + + const response = await auditBrokenBacklinks(message, context); + + expect(response.status).to.equal(500); + expect(mockLog.error).to.have.been.calledWith('Fetching opportunities for siteId site1 failed with error: oppty error'); + }); + + it('should return 500 when opportunity saving fails', async () => { + mockDataAccess.getSiteByID = sinon.stub().withArgs('site1').resolves(site); + mockDataAccess.getTopPagesForSite.resolves([]); + mockDataAccess.getConfiguration = sinon.stub().resolves(configuration); + brokenBacklinksOpportunity.setAuditId.returns(brokenBacklinksOpportunity); + brokenBacklinksOpportunity.save.rejects(new Error('save error')); + mockDataAccess.Opportunity.allBySiteIdAndStatus.resolves( + [otherOpportunity, brokenBacklinksOpportunity], + ); + + nock(site.getBaseURL()) + .get(/.*/) + .reply(200); + + nock('https://ahrefs.com') + .get(/.*/) + .reply(200, auditResult); + + const response = await auditBrokenBacklinks(message, context); + + expect(response.status).to.equal(500); + expect(mockLog.error).to.have.been.calledWith('Creating opportunity for siteId site1 failed with error: save error'); + }); + it('returns a 200 when site is not live', async () => { const siteWithDisabledAudits = createSite({ ...siteData, @@ -665,6 +935,11 @@ describe('Backlinks Tests', function () { it('should handle audit api errors gracefully', async () => { mockDataAccess.getSiteByID = sinon.stub().withArgs('site1').resolves(site); mockDataAccess.getConfiguration = sinon.stub().resolves(configuration); + brokenBacklinksOpportunity.setAuditId.returns(brokenBacklinksOpportunity); + brokenBacklinksOpportunity.addSuggestions.resolves(brokenBacklinksSuggestions); + mockDataAccess.Opportunity.allBySiteIdAndStatus.resolves( + [otherOpportunity, brokenBacklinksOpportunity], + ); nock(site.getBaseURL()) .get(/.*/) diff --git a/test/utils/data-access.test.js b/test/utils/data-access.test.js index 50033d37..07a9ac5e 100644 --- a/test/utils/data-access.test.js +++ b/test/utils/data-access.test.js @@ -15,54 +15,175 @@ import { expect, use } from 'chai'; import chaiAsPromised from 'chai-as-promised'; import sinon from 'sinon'; -import { retrieveSiteBySiteId } from '../../src/utils/data-access.js'; +import sinonChai from 'sinon-chai'; +import { retrieveSiteBySiteId, syncSuggestions } from '../../src/utils/data-access.js'; +use(sinonChai); use(chaiAsPromised); -describe('retrieveSiteBySiteId', () => { - let mockDataAccess; - let mockLog; +describe('data-access', () => { + describe('retrieveSiteBySiteId', () => { + let mockDataAccess; + let mockLog; - beforeEach(() => { - mockDataAccess = { - getSiteByID: sinon.stub(), - }; + beforeEach(() => { + mockDataAccess = { + getSiteByID: sinon.stub(), + }; - mockLog = { - warn: sinon.spy(), - }; - }); + mockLog = { + warn: sinon.spy(), + }; + }); - afterEach(() => { - sinon.restore(); - }); + afterEach(() => { + sinon.restore(); + }); - it('returns site when getSiteByID returns a valid object', async () => { - const site = { id: 'site1' }; - mockDataAccess.getSiteByID.resolves(site); + it('returns site when getSiteByID returns a valid object', async () => { + const site = { id: 'site1' }; + mockDataAccess.getSiteByID.resolves(site); - const result = await retrieveSiteBySiteId(mockDataAccess, 'site1', mockLog); + const result = await retrieveSiteBySiteId(mockDataAccess, 'site1', mockLog); - expect(result).to.equal(site); - expect(mockDataAccess.getSiteByID).to.have.been.calledOnceWith('site1'); - expect(mockLog.warn).to.not.have.been.called; - }); + expect(result).to.equal(site); + expect(mockDataAccess.getSiteByID).to.have.been.calledOnceWith('site1'); + expect(mockLog.warn).to.not.have.been.called; + }); - it('returns null and logs a warning when getSiteByID returns a non-object', async () => { - mockDataAccess.getSiteByID.resolves('not an object'); + it('returns null and logs a warning when getSiteByID returns a non-object', async () => { + mockDataAccess.getSiteByID.resolves('not an object'); - const result = await retrieveSiteBySiteId(mockDataAccess, 'site1', mockLog); + const result = await retrieveSiteBySiteId(mockDataAccess, 'site1', mockLog); - expect(result).to.be.null; - expect(mockDataAccess.getSiteByID).to.have.been.calledOnceWith('site1'); - expect(mockLog.warn).to.have.been.calledOnceWith('Site not found for site: site1'); + expect(result).to.be.null; + expect(mockDataAccess.getSiteByID).to.have.been.calledOnceWith('site1'); + expect(mockLog.warn).to.have.been.calledOnceWith('Site not found for site: site1'); + }); + + it('throws an error when getSiteByID throws an error', async () => { + mockDataAccess.getSiteByID.rejects(new Error('database error')); + + await expect(retrieveSiteBySiteId(mockDataAccess, 'site1', mockLog)).to.be.rejectedWith('Error getting site site1: database error'); + expect(mockDataAccess.getSiteByID).to.have.been.calledOnceWith('site1'); + expect(mockLog.warn).to.not.have.been.called; + }); }); - it('throws an error when getSiteByID throws an error', async () => { - mockDataAccess.getSiteByID.rejects(new Error('database error')); + describe('syncSuggestions', () => { + let mockOpportunity; + let mockLogger; + + beforeEach(() => { + mockOpportunity = { + getSuggestions: sinon.stub(), + addSuggestions: sinon.stub(), + getSiteId: () => 'site-id', + }; + + mockLogger = { + error: sinon.spy(), + info: sinon.spy(), + }; + }); + + it('should remove outdated suggestions and add new ones', async () => { + const existingSuggestions = [{ id: '1', remove: sinon.stub() }, { id: '2', remove: sinon.stub() }]; + const newData = [{ id: '3' }, { id: '4' }]; + const buildKey = sinon.stub().callsFake((item) => item.id); + const mapNewSuggestion = sinon.stub().callsFake((item) => item); + + mockOpportunity.getSuggestions.resolves(existingSuggestions); + mockOpportunity.addSuggestions.resolves({ errorItems: [], createdItems: newData }); + + await syncSuggestions({ + opportunity: mockOpportunity, + newData, + buildKey, + mapNewSuggestion, + log: mockLogger, + }); + + expect(mockOpportunity.getSuggestions).to.have.been.calledOnce; + expect(buildKey.callCount).to.equal(14); + expect(mapNewSuggestion.callCount).to.equal(2); + expect(mockOpportunity.addSuggestions).to.have.been.calledOnceWith(newData); + expect(mockLogger.error).to.not.have.been.called; + }); + + it('should update suggestions when they are detected again', async () => { + const existingSuggestions = [{ + id: '1', title: 'old title', setData: sinon.stub(), save: sinon.stub(), + }, { id: '2', title: 'same title', remove: sinon.stub() }]; + const newData = [{ id: '1', title: 'new title' }]; + const buildKey = sinon.stub().callsFake((item) => item.id); + const mapNewSuggestion = sinon.stub().callsFake((item) => item); + + mockOpportunity.getSuggestions.resolves(existingSuggestions); + mockOpportunity.addSuggestions.resolves({ errorItems: [], createdItems: newData }); + + await syncSuggestions({ + opportunity: mockOpportunity, + newData, + buildKey, + mapNewSuggestion, + log: mockLogger, + }); + + expect(mockOpportunity.getSuggestions).to.have.been.calledOnce; + expect(existingSuggestions[0].setData).to.have.been.calledOnceWith(newData[0]); + expect(existingSuggestions[0].save).to.have.been.calledOnce; + expect(existingSuggestions[1].remove).to.have.been.calledOnce; + }); + + it('should log errors if there are items with errors', async () => { + const existingSuggestions = [{ id: '1', remove: sinon.stub() }]; + const newData = [{ id: '2' }]; + const buildKey = sinon.stub().callsFake((item) => item.id); + const mapNewSuggestion = sinon.stub().callsFake((item) => item); + + mockOpportunity.getSuggestions.resolves(existingSuggestions); + mockOpportunity.addSuggestions.resolves({ + errorItems: [{ item: { id: '2' }, error: 'some error' }], + createdItems: [], + }); + + try { + await syncSuggestions({ + opportunity: mockOpportunity, + newData, + buildKey, + mapNewSuggestion, + log: mockLogger, + }); + } catch (e) { + expect(e.message).to.equal('Failed to create suggestions for siteId site-id'); + } + + expect(mockLogger.error).to.have.been.calledTwice; + expect(mockLogger.error.firstCall.args[0]).to.include('contains 1 items with errors'); + expect(mockLogger.error.secondCall.args[0]).to.include('failed with error: some error'); + }); + + it('should throw an error if all items fail to be created', async () => { + const existingSuggestions = [{ id: '1', remove: sinon.stub() }]; + const newData = [{ id: '2' }]; + const buildKey = sinon.stub().callsFake((item) => item.id); + const mapNewSuggestion = sinon.stub().callsFake((item) => item); + + mockOpportunity.getSuggestions.resolves(existingSuggestions); + mockOpportunity.addSuggestions.resolves({ + errorItems: [{ item: { id: '2' }, error: 'some error' }], + createdItems: [], + }); - await expect(retrieveSiteBySiteId(mockDataAccess, 'site1', mockLog)).to.be.rejectedWith('Error getting site site1: database error'); - expect(mockDataAccess.getSiteByID).to.have.been.calledOnceWith('site1'); - expect(mockLog.warn).to.not.have.been.called; + await expect(syncSuggestions({ + opportunity: mockOpportunity, + newData, + buildKey, + mapNewSuggestion, + log: mockLogger, + })).to.be.rejectedWith('Failed to create suggestions for siteId'); + }); }); });