From c23dca460e3fb01627431654a23db8a7d3bec756 Mon Sep 17 00:00:00 2001 From: Michael Levin Date: Thu, 22 Aug 2024 16:03:55 -0400 Subject: [PATCH] [Bugfix] Pass agency and hostname to each data processor method call --- index.js | 2 +- .../process_google_analytics_results.js | 12 +- src/logger.js | 9 +- .../analytics_data_processor.js | 36 +++--- src/processor.js | 2 +- src/publish/disk.js | 12 +- .../process_google_analytics_results.test.js | 15 ++- .../analytics_data_processor.test.js | 112 +++++++++--------- test/process_results/result_formatter.test.js | 8 +- .../result_totals_calculator.test.js | 18 +-- 10 files changed, 116 insertions(+), 110 deletions(-) diff --git a/index.js b/index.js index 854a1762..6d7afa7f 100644 --- a/index.js +++ b/index.js @@ -129,7 +129,7 @@ async function runQueuePublish(options = {}) { ), { priority: _messagePriority(reportConfig), - singletonKey: `${appConfig.scriptName}-${appConfig.agency}-${reportConfig.name}`, + singletonKey: `${appConfig.scriptName}-${agency.agencyName}-${reportConfig.name}`, }, ); if (jobId) { diff --git a/src/actions/process_google_analytics_results.js b/src/actions/process_google_analytics_results.js index 19e0c20b..8a0666b9 100644 --- a/src/actions/process_google_analytics_results.js +++ b/src/actions/process_google_analytics_results.js @@ -26,11 +26,13 @@ class ProcessGoogleAnalyticsResults extends Action { async executeStrategy(context) { context.logger.debug("Processing GA report data"); context.processedAnalyticsData = - await this.#analyticsDataProcessor.processData( - context.reportConfig, - context.rawGoogleAnalyticsReportData[0], - context.googleAnalyticsQuery, - ); + await this.#analyticsDataProcessor.processData({ + agency: context.appConfig.agency, + hostname: context.appConfig.account.hostname, + report: context.reportConfig, + data: context.rawGoogleAnalyticsReportData[0], + query: context.googleAnalyticsQuery, + }); } } diff --git a/src/logger.js b/src/logger.js index 7736858c..01126b1f 100644 --- a/src/logger.js +++ b/src/logger.js @@ -1,9 +1,12 @@ const winston = require("winston"); /** - * @param {import('../app_config')} appConfig the application config - * @param {object} reportConfig the name of the report currently being processed - * @param {string} reportConfig.name the name of the report being run for this + * @param {object} params the parameters for the method + * @param {string} params.agencyName the name of the agency for this logger + * instance. + * @param {string} params.reportName the name of the report being run for this + * logger instance. + * @param {string} params.scriptName the name of the script being run for this * logger instance. * @returns {string} a standard tag for the logger to identify the specific * report being processed when writing logs. diff --git a/src/process_results/analytics_data_processor.js b/src/process_results/analytics_data_processor.js index 7d341adb..1149596b 100644 --- a/src/process_results/analytics_data_processor.js +++ b/src/process_results/analytics_data_processor.js @@ -4,8 +4,6 @@ const ResultTotalsCalculator = require("./result_totals_calculator"); * Handles processing operations for raw analytics data. */ class AnalyticsDataProcessor { - #agency; - #hostname; #mapping = { activeUsers: "active_visitors", fileName: "file_name", @@ -36,25 +34,19 @@ class AnalyticsDataProcessor { }; /** - * @param {import('../app_config')} appConfig an instance of the application config class. - * Provides agency and hostname string values. - */ - constructor(appConfig) { - this.#agency = appConfig.account.agency_name; - this.#hostname = appConfig.account.hostname; - } - - /** - * @param {object} report The report object that was requested - * @param {object} data The response object from the Google Analytics Data API - * @param {object} query The query object for the report + * @param {object} params the method parameters + * @param {string} params.agency the agency for the report + * @param {string} params.hostname the hostname for the report + * @param {object} params.report The report object that was requested + * @param {object} params.data The response object from the Google Analytics Data API + * @param {object} params.query The query object for the report * @returns {object} The response data transformed to flatten the data * structure, format dates, and map from GA keys to DAP keys. Data is filtered * as requested in the report object. This object also includes details from * the original report and query. */ - processData(report, data, query) { - let result = this.#initializeResult({ report, data, query }); + processData({ agency, hostname, report, data, query }) { + let result = this.#initializeResult({ agency, report, data, query }); // If you use a filter that results in no data, you get null // back from google and need to protect against it. @@ -77,7 +69,7 @@ class AnalyticsDataProcessor { // Process each row result.data = data.rows.map((row) => { - return this.#processRow({ row, report, data }); + return this.#processRow({ hostname, row, data }); }); result.totals = ResultTotalsCalculator.calculateTotals(result, { @@ -88,10 +80,10 @@ class AnalyticsDataProcessor { return result; } - #initializeResult({ report, data, query }) { + #initializeResult({ agency, report, data, query }) { return { name: report.name, - agency: this.#agency, + agency, sampling: data.metadata?.samplingMetadatas, query: ((query) => { query = Object.assign({}, query); @@ -164,7 +156,7 @@ class AnalyticsDataProcessor { return [date.substr(0, 4), date.substr(4, 2), date.substr(6, 2)].join("-"); } - #processRow({ row, data }) { + #processRow({ hostname, row, data }) { const point = {}; // Iterate through each entry in the object @@ -194,8 +186,8 @@ class AnalyticsDataProcessor { }); } - if (this.#hostname && !("domain" in point)) { - point.domain = this.#hostname; + if (hostname && !("domain" in point)) { + point.domain = hostname; } return point; diff --git a/src/processor.js b/src/processor.js index ae25b25e..c021535e 100644 --- a/src/processor.js +++ b/src/processor.js @@ -59,7 +59,7 @@ class Processor { logger, ), ), - new ProcessGoogleAnalyticsResults(new AnalyticsDataProcessor(appConfig)), + new ProcessGoogleAnalyticsResults(new AnalyticsDataProcessor()), new FormatProcessedAnalyticsData(), new WriteAnalyticsDataToDatabase(new PostgresPublisher(appConfig)), new PublishAnalyticsDataToS3(new S3Service(appConfig)), diff --git a/src/publish/disk.js b/src/publish/disk.js index 50c919e7..64ff521f 100644 --- a/src/publish/disk.js +++ b/src/publish/disk.js @@ -4,12 +4,12 @@ const path = require("path"); /** * Publishes report data to a file on the local filesystem. * - * @param {string} name the name of the file to create. - * @param {string} data the data to write to the file. - * @param {string} format the file extension to use - * @param {string} directory the path for the directory to use for the file - * @param {import('../app_config')} appConfig application config instance. Sets the file - * extension and the path of the file to create. + * @param {object} params the parameters fro the method + * @param {string} params.name the name of the file to create. + * @param {string} params.data the data to write to the file. + * @param {string} params.format the file extension to use + * @param {string} params.directory the path for the directory to use for the + * file */ const publish = async ({ name, data, format, directory }) => { const filename = `${name}.${format}`; diff --git a/test/actions/process_google_analytics_results.test.js b/test/actions/process_google_analytics_results.test.js index 7f8a0d4a..746ddfaf 100644 --- a/test/actions/process_google_analytics_results.test.js +++ b/test/actions/process_google_analytics_results.test.js @@ -18,12 +18,15 @@ describe("ProcessGoogleAnalyticsResults", () => { const googleAnalyticsQuery = { name: "users", realtime: true }; const reportConfig = { name: "foobar" }; const processedAnalyticsData = { fake: "data" }; + const agency = "defense"; + const hostname = "example.gov"; beforeEach(async () => { debugLogSpy.resetHistory(); analyticsDataProcessor.processData.resetHistory(); analyticsDataProcessor.processData.returns(processedAnalyticsData); context = { + appConfig: { agency, account: { hostname } }, rawGoogleAnalyticsReportData: rawGoogleAnalyticsReportData, googleAnalyticsQuery: googleAnalyticsQuery, logger: { debug: debugLogSpy }, @@ -34,11 +37,13 @@ describe("ProcessGoogleAnalyticsResults", () => { it("calls analyticsDataProcessor.processData with the expected params", () => { expect( - analyticsDataProcessor.processData.calledWith( - reportConfig, - rawGoogleAnalyticsReportData[0], - googleAnalyticsQuery, - ), + analyticsDataProcessor.processData.calledWith({ + agency, + hostname, + report: reportConfig, + data: rawGoogleAnalyticsReportData[0], + query: googleAnalyticsQuery, + }), ).to.equal(true); }); diff --git a/test/process_results/analytics_data_processor.test.js b/test/process_results/analytics_data_processor.test.js index 5404ab7b..5cab654c 100644 --- a/test/process_results/analytics_data_processor.test.js +++ b/test/process_results/analytics_data_processor.test.js @@ -27,11 +27,11 @@ describe("AnalyticsDataProcessor", () => { appConfig.account = { hostname: "", }; - subject = new AnalyticsDataProcessor(appConfig); + subject = new AnalyticsDataProcessor(); }); it("should return results with the correct props", () => { - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.name).to.be.a("string"); expect(result.query).to.be.an("object"); expect(result.meta).to.be.an("object"); @@ -44,13 +44,13 @@ describe("AnalyticsDataProcessor", () => { it("should return results with an empty data array if data is undefined or has no rows", () => { data.rows = []; - expect(subject.processData(report, data).data).to.be.empty; + expect(subject.processData({ report, data }).data).to.be.empty; data.rows = undefined; - expect(subject.processData(report, data).data).to.be.empty; + expect(subject.processData({ report, data }).data).to.be.empty; }); it("should delete the query ids for the GA response", () => { - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.query).to.not.have.property("ids"); }); @@ -67,7 +67,7 @@ describe("AnalyticsDataProcessor", () => { }, ]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(Object.keys(result.data[0])).to.deep.equal([ "file_name", "os", @@ -80,7 +80,7 @@ describe("AnalyticsDataProcessor", () => { data.dimensionHeaders = [{ name: "date" }]; data.rows = [{ dimensionValues: [{ value: "20170130" }] }]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.data[0].date).to.equal("2017-01-30"); }); @@ -88,7 +88,7 @@ describe("AnalyticsDataProcessor", () => { data.dimensionHeaders = [{ name: "date" }]; data.rows = [{ dimensionValues: [{ value: "(other)" }] }]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.data[0].date).to.equal("(other)"); }); @@ -118,7 +118,7 @@ describe("AnalyticsDataProcessor", () => { }, ]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.data).to.have.length(2); expect(result.data.map((row) => row.unmapped_column)).to.deep.equal([ "20", @@ -149,7 +149,7 @@ describe("AnalyticsDataProcessor", () => { }, ]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.data).to.have.length(3); }); @@ -179,7 +179,7 @@ describe("AnalyticsDataProcessor", () => { }, ]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.data).to.have.length(2); expect(result.data.map((row) => row.unmapped_column)).to.deep.equal([ "20", @@ -204,7 +204,7 @@ describe("AnalyticsDataProcessor", () => { }, ]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.data[0].unmapped_column).to.be.undefined; }); @@ -222,7 +222,7 @@ describe("AnalyticsDataProcessor", () => { }, ]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.data[0].unmapped_column).to.be.undefined; }); @@ -240,28 +240,32 @@ describe("AnalyticsDataProcessor", () => { }, ]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(Object.keys(result.data[0]).length).to.equal(2); }); - it("should add a hostname to realtime data if a hostname is specified by the appConfig", () => { + it("should add a hostname to realtime data if a hostname is passed", () => { report.realtime = true; - appConfig.account.hostname = "www.example.gov"; + let hostname = "www.example.gov"; - subject = new AnalyticsDataProcessor(appConfig); + subject = new AnalyticsDataProcessor(); - const result = subject.processData(report, data); + const result = subject.processData({ report, data, hostname }); expect(result.data[0].domain).to.equal("www.example.gov"); }); - it("should not overwrite the domain with a hostname from the appConfig", () => { + it("should not overwrite the domain with a passed hostname", () => { let dataWithHostname; dataWithHostname = Object.assign({}, dataWithHostnameFixture); report.realtime = true; - appConfig.account.hostname = "www.example.gov"; - subject = new AnalyticsDataProcessor(appConfig); - - const result = subject.processData(report, dataWithHostname); + let hostname = "www.example.gov"; + subject = new AnalyticsDataProcessor(); + + const result = subject.processData({ + report, + data: dataWithHostname, + hostname, + }); expect(result.data[0].domain).to.equal("www.example0.com"); }); @@ -275,14 +279,14 @@ describe("AnalyticsDataProcessor", () => { "../../src/process_results/analytics_data_processor", { "./result_totals_calculator": { calculateTotals } }, ); - subject = new AnalyticsDataProcessor(appConfig); + subject = new AnalyticsDataProcessor(); - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.totals).to.deep.equal({ visits: 1234 }); }); }); - describe("when an agency is set in appConfig", () => { + describe("when an agency is passed", () => { let agency = "interior"; let report; let data; @@ -291,15 +295,11 @@ describe("AnalyticsDataProcessor", () => { beforeEach(() => { report = Object.assign({}, reportFixture); data = Object.assign({}, dataFixture); - appConfig.account = { - agency_name: agency, - hostname: "", - }; - subject = new AnalyticsDataProcessor(appConfig); + subject = new AnalyticsDataProcessor(); }); it("should return results with the correct props", () => { - const result = subject.processData(report, data); + const result = subject.processData({ report, data, agency }); expect(result.name).to.be.a("string"); expect(result.query).to.be.an("object"); expect(result.meta).to.be.an("object"); @@ -312,13 +312,13 @@ describe("AnalyticsDataProcessor", () => { it("should return results with an empty data array if data is undefined or has no rows", () => { data.rows = []; - expect(subject.processData(report, data).data).to.be.empty; + expect(subject.processData({ report, data }).data).to.be.empty; data.rows = undefined; - expect(subject.processData(report, data).data).to.be.empty; + expect(subject.processData({ report, data }).data).to.be.empty; }); it("should delete the query ids for the GA response", () => { - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.query).to.not.have.property("ids"); }); @@ -335,7 +335,7 @@ describe("AnalyticsDataProcessor", () => { }, ]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(Object.keys(result.data[0])).to.deep.equal([ "file_name", "os", @@ -348,7 +348,7 @@ describe("AnalyticsDataProcessor", () => { data.dimensionHeaders = [{ name: "date" }]; data.rows = [{ dimensionValues: [{ value: "20170130" }] }]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.data[0].date).to.equal("2017-01-30"); }); @@ -356,7 +356,7 @@ describe("AnalyticsDataProcessor", () => { data.dimensionHeaders = [{ name: "date" }]; data.rows = [{ dimensionValues: [{ value: "(other)" }] }]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.data[0].date).to.equal("(other)"); }); @@ -386,7 +386,7 @@ describe("AnalyticsDataProcessor", () => { }, ]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.data).to.have.length(2); expect(result.data.map((row) => row.unmapped_column)).to.deep.equal([ "20", @@ -417,7 +417,7 @@ describe("AnalyticsDataProcessor", () => { }, ]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.data).to.have.length(3); }); @@ -447,7 +447,7 @@ describe("AnalyticsDataProcessor", () => { }, ]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.data).to.have.length(2); expect(result.data.map((row) => row.unmapped_column)).to.deep.equal([ "20", @@ -472,7 +472,7 @@ describe("AnalyticsDataProcessor", () => { }, ]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.data[0].unmapped_column).to.be.undefined; }); @@ -490,7 +490,7 @@ describe("AnalyticsDataProcessor", () => { }, ]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.data[0].unmapped_column).to.be.undefined; }); @@ -508,28 +508,32 @@ describe("AnalyticsDataProcessor", () => { }, ]; - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(Object.keys(result.data[0]).length).to.equal(2); }); it("should add a hostname to realtime data if a hostname is specified by the appConfig", () => { report.realtime = true; - appConfig.account.hostname = "www.example.gov"; + let hostname = "www.example.gov"; - subject = new AnalyticsDataProcessor(appConfig); + subject = new AnalyticsDataProcessor(); - const result = subject.processData(report, data); + const result = subject.processData({ report, data, hostname }); expect(result.data[0].domain).to.equal("www.example.gov"); }); - it("should not overwrite the domain with a hostname from the appConfig", () => { + it("should not overwrite the domain with a passed hostname", () => { let dataWithHostname; dataWithHostname = Object.assign({}, dataWithHostnameFixture); report.realtime = true; - appConfig.account.hostname = "www.example.gov"; - subject = new AnalyticsDataProcessor(appConfig); - - const result = subject.processData(report, dataWithHostname); + let hostname = "www.example.gov"; + subject = new AnalyticsDataProcessor(); + + const result = subject.processData({ + report, + data: dataWithHostname, + hostname, + }); expect(result.data[0].domain).to.equal("www.example0.com"); }); @@ -543,9 +547,9 @@ describe("AnalyticsDataProcessor", () => { "../../src/process_results/analytics_data_processor", { "./result_totals_calculator": { calculateTotals } }, ); - subject = new AnalyticsDataProcessor(appConfig); + subject = new AnalyticsDataProcessor(); - const result = subject.processData(report, data); + const result = subject.processData({ report, data }); expect(result.totals).to.deep.equal({ visits: 1234 }); }); }); diff --git a/test/process_results/result_formatter.test.js b/test/process_results/result_formatter.test.js index c45acf12..71f1de7b 100644 --- a/test/process_results/result_formatter.test.js +++ b/test/process_results/result_formatter.test.js @@ -25,7 +25,7 @@ describe("ResultFormatter", () => { }); it("should format results into JSON if the format is 'json'", (done) => { - const result = analyticsDataProcessor.processData(report, data); + const result = analyticsDataProcessor.processData({ report, data }); ResultFormatter.formatResult(result, { format: "json" }) .then((formattedResult) => { @@ -37,7 +37,7 @@ describe("ResultFormatter", () => { }); it("should remove the data attribute for JSON if options.slim is true", (done) => { - const result = analyticsDataProcessor.processData(report, data); + const result = analyticsDataProcessor.processData({ report, data }); ResultFormatter.formatResult(result, { format: "json", slim: true }) .then((formattedResult) => { @@ -62,7 +62,7 @@ describe("ResultFormatter", () => { }); it("should format results into CSV if the format is 'csv'", (done) => { - const result = analyticsDataProcessor.processData(report, data); + const result = analyticsDataProcessor.processData({ report, data }); ResultFormatter.formatResult(result, { format: "csv", @@ -84,7 +84,7 @@ describe("ResultFormatter", () => { }); it("should throw an error if the format is unsupported", (done) => { - const result = analyticsDataProcessor.processData(report, data); + const result = analyticsDataProcessor.processData({ report, data }); ResultFormatter.formatResult(result, { format: "xml", diff --git a/test/process_results/result_totals_calculator.test.js b/test/process_results/result_totals_calculator.test.js index c8ec577f..89ce84e8 100644 --- a/test/process_results/result_totals_calculator.test.js +++ b/test/process_results/result_totals_calculator.test.js @@ -32,7 +32,7 @@ describe("ResultTotalsCalculator", () => { }); it("should return an empty object", () => { - const result = analyticsDataProcessor.processData(report, data); + const result = analyticsDataProcessor.processData({ report, data }); const totals = ResultTotalsCalculator.calculateTotals(result); expect(totals).to.eql({}); @@ -57,7 +57,7 @@ describe("ResultTotalsCalculator", () => { { metricValues: [{ value: "15" }] }, { metricValues: [{ value: "20" }] }, ]; - const result = analyticsDataProcessor.processData(report, data); + const result = analyticsDataProcessor.processData({ report, data }); totals = ResultTotalsCalculator.calculateTotals(result); }); @@ -75,7 +75,7 @@ describe("ResultTotalsCalculator", () => { { metricValues: [{ value: "20" }] }, ]; - const result = analyticsDataProcessor.processData(report, data); + const result = analyticsDataProcessor.processData({ report, data }); totals = ResultTotalsCalculator.calculateTotals(result); }); @@ -123,7 +123,7 @@ describe("ResultTotalsCalculator", () => { }, ]; - const result = analyticsDataProcessor.processData(report, data); + const result = analyticsDataProcessor.processData({ report, data }); totals = ResultTotalsCalculator.calculateTotals(result, options); }); @@ -195,7 +195,7 @@ describe("ResultTotalsCalculator", () => { }, ]; - const result = analyticsDataProcessor.processData(report, data); + const result = analyticsDataProcessor.processData({ report, data }); totals = ResultTotalsCalculator.calculateTotals(result, options); }); @@ -256,7 +256,7 @@ describe("ResultTotalsCalculator", () => { }, ]; - const result = analyticsDataProcessor.processData(report, data); + const result = analyticsDataProcessor.processData({ report, data }); totals = ResultTotalsCalculator.calculateTotals(result, options); }); @@ -328,7 +328,7 @@ describe("ResultTotalsCalculator", () => { }, ]; - const result = analyticsDataProcessor.processData(report, data); + const result = analyticsDataProcessor.processData({ report, data }); totals = ResultTotalsCalculator.calculateTotals(result, options); }); @@ -426,7 +426,7 @@ describe("ResultTotalsCalculator", () => { }, ]; - const result = analyticsDataProcessor.processData(report, data); + const result = analyticsDataProcessor.processData({ report, data }); const totals = ResultTotalsCalculator.calculateTotals(result); @@ -512,7 +512,7 @@ describe("ResultTotalsCalculator", () => { }, ]; - const result = analyticsDataProcessor.processData(report, data); + const result = analyticsDataProcessor.processData({ report, data }); const totals = ResultTotalsCalculator.calculateTotals(result);