diff --git a/memory-bank/change-log.md b/memory-bank/change-log.md index 98e99d86..f0fad0e1 100644 --- a/memory-bank/change-log.md +++ b/memory-bank/change-log.md @@ -620,3 +620,72 @@ Validation: Follow-ups: - Optional: run full repo lint in a separate pass for broader non-slice hygiene now that targeted contract suite is stable. + +--- + +### CL-016: TASK22224 documents download contract slice + +date: 2026-03-24 +author: Cline +scope: `pages/api/documents/download/[id].js`, `tests/phase21/{documents-handler-contract,api-contract-slice1}.test.cjs` +type: change +rationale: Execute next aggressive slice by standardizing document download guard behavior and bringing the route under phase21 contract coverage. +impact: Improves reliability on invalid input and relay-failure paths while preserving existing user-visible fallback behavior (`/filenotavailable`) for download failures. +status: completed + +Summary: + +- `pages/api/documents/download/[id].js` + - added explicit required-query guard for `id` and `hash` + - unified fallback redirect path via constant (`/filenotavailable`) + - preserved streaming download behavior and retry flow +- added `tests/phase21/documents-handler-contract.test.cjs` covering: + - missing query -> redirect contract + - success -> attachment/content-type headers + stream pipe contract + - relay failure -> redirect contract +- updated combined runner (`tests/phase21/api-contract-slice1.test.cjs`) to include documents handler contract suite + +Validation: + +- `node tests/phase21/api-contract-slice1.test.cjs` -> pass + - helper: 4/4 + - file-handler: 53/53 + - email-handler: 12/12 + - endpoint-handler: 152/152 + - documents-handler: 3/3 + +Follow-ups: + +- Optional future hardening: migrate documents route onto shared `respondError/respondSuccess` envelope if product requirements allow replacing redirect-style fallback. + +--- + +### CL-017: TASK22224 endpoint legacy-comment hygiene slice + +date: 2026-03-24 +author: Cline +scope: `pages/api/endpoint/{createwatchedcases_api,getadvancedsearchpaged_api}.js` +type: change +rationale: Complete second requested slice with low-risk maintainability cleanup by removing large obsolete commented legacy handler blocks. +impact: Non-behavioral cleanup only; improves readability and reduces maintenance noise with no runtime contract changes. +status: completed + +Summary: + +- `createwatchedcases_api.js` + - removed obsolete commented promise-chain implementation block +- `getadvancedsearchpaged_api.js` + - removed obsolete commented legacy implementation block retained below active handler + +Validation: + +- `node tests/phase21/api-contract-slice1.test.cjs` -> pass + - helper: 4/4 + - file-handler: 53/53 + - email-handler: 12/12 + - endpoint-handler: 152/152 + - documents-handler: 3/3 + +Follow-ups: + +- Optional further hygiene pass can target remaining oversized commented historical sections in non-sensitive handlers. diff --git a/pages/api/documents/download/[id].js b/pages/api/documents/download/[id].js index aab6804b..4fbd0752 100644 --- a/pages/api/documents/download/[id].js +++ b/pages/api/documents/download/[id].js @@ -116,6 +116,8 @@ const WEBAPI_URL = process.env.RELAY_ROOT || "https://dev-pedw-ns.servicebus.windows.net/dev-pedw-hc/"; +const FILE_NOT_AVAILABLE_PATH = "/filenotavailable"; + // Retry utility with logging async function retry(fn, retries = 3, delay = 1000) { for (let attempt = 1; attempt <= retries; attempt++) { @@ -133,6 +135,18 @@ async function retry(fn, retries = 3, delay = 1000) { const ApiProxy = async (req, res) => { const docRef = req.query; + if ( + typeof docRef?.id !== "string" || + docRef.id.length === 0 || + typeof docRef?.hash !== "string" || + docRef.hash.length === 0 + ) { + if (!res.headersSent) { + res.redirect(FILE_NOT_AVAILABLE_PATH); + } + return; + } + try { const token = await getToken(); const startTime = Date.now(); @@ -186,11 +200,11 @@ const ApiProxy = async (req, res) => { response.data.on("error", (err) => { consoleLogger(err); - if (!res.headersSent) res.redirect("/filenotavailable"); + if (!res.headersSent) res.redirect(FILE_NOT_AVAILABLE_PATH); }); } catch (error) { consoleLogger(error); - if (!res.headersSent) res.redirect("/filenotavailable"); + if (!res.headersSent) res.redirect(FILE_NOT_AVAILABLE_PATH); } }; diff --git a/pages/api/endpoint/createwatchedcases_api.js b/pages/api/endpoint/createwatchedcases_api.js index 55662558..da199830 100644 --- a/pages/api/endpoint/createwatchedcases_api.js +++ b/pages/api/endpoint/createwatchedcases_api.js @@ -20,35 +20,6 @@ const WEBAPI_URL = process.env.RELAY_ROOT || "https://dev-pedw-ns.servicebus.windows.net/dev-pedw-hc/"; -// export default async function ApiProxy(req, res) { -// var token = await getToken(); -// var data = JSON.stringify(req.body); -// var queryUrl = "pinswg_watchlists"; - -// console.log(data); -// var config = { -// method: "put", -// url: WEBAPI_URL + queryUrl + hashAPIPath(queryUrl), -// headers: { -// "OData-MaxVersion": "4.0", -// "OData-Version": "4.0", -// "Accept": "application/json", -// "Prefer": 'odata.include-annotations="*",return=representation', -// "Authorization": "Bearer " + token.access_token, -// "Content-Type": "application/json", -// }, -// data: data, -// }; - -// return axios(config) -// .then(({ data }) => { -// res.status(200).json(data); -// }) -// .catch((error) => { -// consoleLogger(Object.assign(err, data)); -// res.status(400).json(error); -// }); -// } const recordExists = async (incidentId, contactId, token) => { const filter = `$filter=pinswg_WatchedCase/incidentid eq ${incidentId} and pinswg_Contact/contactid eq ${contactId}`; const queryUrl = `pinswg_watchlists?${filter}`; diff --git a/pages/api/endpoint/getadvancedsearchpaged_api.js b/pages/api/endpoint/getadvancedsearchpaged_api.js index 915efdeb..3150880e 100644 --- a/pages/api/endpoint/getadvancedsearchpaged_api.js +++ b/pages/api/endpoint/getadvancedsearchpaged_api.js @@ -258,161 +258,3 @@ export default async function ApiProxy(req, res) { }); } } - -// export default async function ApiProxy(req, res) { -// var searchString = req.query.searchstring; -// var pageNumber = req.query.pageNumber; -// var token = await getToken(); - -// var orderby = req.query.orderby; -// var fieldSort = req.query.fieldSort; -// var showNumberOfRecords = req.query.showNumberOfRecords; - -// searchString = _.isEmpty(searchString) -// ? searchString -// : JSON.parse(decodeURI(searchString)); - -// var queryString = ""; - -// queryString += -// _.has(searchString, "q") && searchString.q != null -// ? "(contains(title, '" + -// searchString.q.replace(/\'/g, "''") + -// "') or contains(ticketnumber,'" + -// searchString.q.replace(/\'/g, "''") + -// "')) and" -// : ""; - -// queryString += -// (_.has(searchString, "lpa") || _.has(searchString, "LPA")) && -// searchString.lpa != null -// ? " _pinswg_associatedlpa_value eq " + searchString.lpa + " and" -// : ""; - -// queryString += -// _.has(searchString, "apt") && searchString.apt != null -// ? " pinswg_appealcasetype eq " + searchString.apt + " and" -// : ""; - -// queryString += -// _.has(searchString, "statuscode") && searchString.statuscode != null -// ? " statuscode eq " + searchString.statuscode + " and" -// : ""; - -// var queryUrl = -// "incidents?$select=pinswg_environmentalstatementlocation,pinswg_caseaddress,modifiedon,description,numberofchildincidents,_accountid_value,_customerid_value,_pinswg_associatedlpa_value,_ownerid_value,pinswg_appealcasetype,statuscode,ticketnumber,title,_primarycontactid_value,pinswg_lpareference,pinswg_appellantagent,pinswg_appellantfirstname,pinswg_appellantlastname&$expand=primarycontactid($select=fullname)&$filter=" + -// queryString + -// " pinswg_appealcasetype ne null and pinswg_publishtoweb eq true&$orderby=" + -// orderby + -// " " + -// fieldSort + -// "&$count=true"; - -// console.log(WEBAPI_URL + queryUrl + hashAPIPath(queryUrl)); - -// var apiResponse = _.isEmpty(searchString) -// ? res.status(400).json() -// : axios -// .get( -// WEBAPI_URL + queryUrl + hashAPIPath(queryUrl), -// azureHeadersPagedCustom( -// token.access_token, -// showNumberOfRecords -// ) -// ) -// .then(async ({ data }) => { -// var dataStr; -// _.has(data, "@odata.nextLink") == true && -// ((dataStr = JSON.stringify(data["@odata.nextLink"])), -// (data["@odata.nextLink"] = dataStr.split("/v8.2/")[1])); - -// if (_.has(searchString, "projecttype")) { -// await updateValueArray(data, token); - -// async function updateValueArray(data, token) { -// let hasNextPage = true; -// let nextPageUrl = data["@odata.nextLink"]; - -// // Loop through all pages -// while (hasNextPage) { -// // Iterate through current data and process it -// for (let i = 0; i < data.value.length; i++) { -// const item = data.value[i]; - -// try { -// // Axios call using the `incidentid` to fetch additional data -// const response = await axios.get( -// WEBAPI_URL + -// "pinswg_sipses?$filter=_pinswg_sipscase_value eq " + -// item.incidentid + -// " and _pinswg_projecttype_value eq " + -// searchString.projecttype + -// "&$select=_pinswg_projecttype_value" + -// hashAPIPath( -// "pinswg_sipses?$filter=_pinswg_sipscase_value eq " + -// item.incidentid + -// " and _pinswg_projecttype_value eq " + -// searchString.projecttype + -// "&$select=_pinswg_projecttype_value" -// ), -// azureHeadersPaged(token.access_token) -// ); - -// if ( -// response.data.value.length > 0 && -// response.data.value[0] -// ._pinswg_projecttype_value != null -// ) { -// Object.assign( -// item, -// response.data.value[0] -// ); // Update item with new data -// } else { -// console.log( -// `No project type found for incident ID ${item.incidentid}` -// ); -// } -// } catch (error) { -// console.error( -// `Error fetching data for incident ID ${item.incidentid}:`, -// error -// ); -// } -// } - -// // If there's a next page, fetch it -// if (nextPageUrl) { -// const nextPageData = await axios.get( -// WEBAPI_URL + -// nextPageUrl + -// hashAPIPath(nextPageUrl), -// azureHeadersPaged(token.access_token) -// ); -// data.value = [ -// ...data.value, -// ...nextPageData.data.value, -// ]; -// nextPageUrl = -// nextPageData.data["@odata.nextLink"]; -// } else { -// hasNextPage = false; -// } -// } -// } - -// data.value = data.value.filter( -// (item) => item._pinswg_projecttype_value != null -// ); - -// data["@odata.count"] = data.value.length; -// } - -// res.status(200).json(data); -// }) -// .catch((error) => { -// consoleLogger(error); -// res.status(400).json(error); -// }); - -// return apiResponse; -// } diff --git a/tests/phase21/api-contract-slice1.test.cjs b/tests/phase21/api-contract-slice1.test.cjs index 2f89aaf8..f055de07 100644 --- a/tests/phase21/api-contract-slice1.test.cjs +++ b/tests/phase21/api-contract-slice1.test.cjs @@ -2,12 +2,14 @@ const runHelperTests = require("./api-response-helper.test.cjs"); const runHandlerTests = require("./file-handler-contract.test.cjs"); const runEmailHandlerTests = require("./email-handler-contract.test.cjs"); const runEndpointHandlerTests = require("./endpoint-handler-contract.test.cjs"); +const runDocumentsHandlerTests = require("./documents-handler-contract.test.cjs"); const run = async () => { await runHelperTests(); await runHandlerTests(); await runEmailHandlerTests(); await runEndpointHandlerTests(); + await runDocumentsHandlerTests(); console.log("Phase 21 combined suite passed."); }; diff --git a/tests/phase21/documents-handler-contract.test.cjs b/tests/phase21/documents-handler-contract.test.cjs new file mode 100644 index 00000000..e1035b3e --- /dev/null +++ b/tests/phase21/documents-handler-contract.test.cjs @@ -0,0 +1,134 @@ +const assert = require("assert"); +const { Readable } = require("stream"); +const { loadModule } = require("./_shared.cjs"); + +const tests = []; +const test = (name, fn) => tests.push({ name, fn }); + +const createRedirectRes = () => { + const state = { + redirectedTo: null, + headers: {}, + piped: false, + headersSent: false + }; + + return { + state, + get headersSent() { + return state.headersSent; + }, + setHeader(key, value) { + state.headers[key.toLowerCase()] = value; + }, + redirect(path) { + state.redirectedTo = path; + state.headersSent = true; + return path; + } + }; +}; + +test("documents download redirects when id/hash query is missing", async () => { + const mod = loadModule("pages/api/documents/download/[id].js", { + getToken: async () => ({ access_token: "tok" }), + consoleLogger: () => {}, + axios: { get: async () => ({}) } + }); + + const req = { query: { id: "DOC-1" } }; + const res = createRedirectRes(); + + await mod.default(req, res); + + assert.strictEqual(res.state.redirectedTo, "/filenotavailable"); +}); + +test("documents download success sets attachment headers and pipes stream", async () => { + const stream = new Readable({ + read() {} + }); + + const mod = loadModule("pages/api/documents/download/[id].js", { + setTimeout: (fn) => { + fn(); + return 0; + }, + getToken: async () => ({ access_token: "tok" }), + consoleLogger: () => {}, + axios: { + get: async () => ({ + headers: { + "content-disposition": "attachment; filename=test-file.pdf" + }, + data: stream + }) + } + }); + + const req = { query: { id: "DOC-1", hash: "h1" } }; + const res = createRedirectRes(); + + stream.pipe = (target) => { + target.state.piped = true; + target.state.headersSent = true; + return target; + }; + + await mod.default(req, res); + + assert.strictEqual( + res.state.headers["content-disposition"], + "attachment; filename=test-file.pdf" + ); + assert.strictEqual( + res.state.headers["content-type"], + "application/octet-stream" + ); + assert.strictEqual(res.state.piped, true); + assert.strictEqual(res.state.redirectedTo, null); +}); + +test("documents download redirects when relay request fails", async () => { + const mod = loadModule("pages/api/documents/download/[id].js", { + setTimeout: (fn) => { + fn(); + return 0; + }, + getToken: async () => ({ access_token: "tok" }), + consoleLogger: () => {}, + axios: { + get: async () => { + throw new Error("relay failed"); + } + } + }); + + const req = { query: { id: "DOC-1", hash: "h1" } }; + const res = createRedirectRes(); + + await mod.default(req, res); + + assert.strictEqual(res.state.redirectedTo, "/filenotavailable"); +}); + +const run = async () => { + let passed = 0; + for (const currentTest of tests) { + await currentTest.fn(); + passed += 1; + } + + console.log( + `Phase 21 documents-handler contract tests passed (${passed}/${tests.length}).` + ); +}; + +module.exports = run; + +if (require.main === module) { + run().catch((error) => { + console.error(error); + process.exit(1); + }); +}