[DRAFT][FOR-REFERENCE-ONLY]: Save sold-land-filter option to database
#1454
Conversation
…DIS_removal flag is turned on
…ing or updating an overseas entity
…tity when saving OE name
mwejuli-ch
force-pushed
the
feat/roe-2559/save-sold-land-filter-option-to-database
branch
from
0ab0482 to
5c3d9a3
Compare
|
|
||
| if (!response.httpStatusCode || response.httpStatusCode >= 400) { | ||
| throw createAndLogErrorRequest(req, `'postTransaction' for company number '${companyNumber}' with name '${companyName}' returned HTTP status code ${response.httpStatusCode}`); | ||
| throw createAndLogErrorRequest(req, `'>>error:>>>>>>>>postTransaction' for company number '${companyNumber}' with name '${companyName}' returned HTTP status code ${response.httpStatusCode}`); |
There was a problem hiding this comment.
The >>error:>>>>>>>> text here and below will need to go.
There was a problem hiding this comment.
Yes, will get rid of these. The new PR doesn't have them.
| import { isActiveFeature } from "./feature.flag"; | ||
| import { getOverseasEntity } from "../service/overseas.entities.service"; | ||
|
|
||
| export const getApplicationData = async (session: Session | undefined, req?: Request): Promise<ApplicationData> => { |
There was a problem hiding this comment.
I assume at some point this function will no longer take a session as parameter? Retrieval will always be based on a transaction and submission id?
There was a problem hiding this comment.
Absolutely. When we have an entirely REDIS-free journey, this method will need not have this parameter.
src/utils/application.data.ts
Outdated
| import { getOverseasEntity } from "../service/overseas.entities.service"; | ||
|
|
||
| export const getApplicationData = async (session: Session | undefined, req?: Request): Promise<ApplicationData> => { | ||
| if (typeof req === "undefined" || !isActiveFeature(FEATURE_FLAG_ENABLE_REDIS_REMOVAL)) { |
There was a problem hiding this comment.
Similarly, perhaps in the future, when work is nearer complete, if the request is undefined I think this should be an error.
| const submissionId: string = req.params[ROUTE_PARAM_SUBMISSION_ID] ?? ""; | ||
|
|
||
| if (transactionId === "" && submissionId === "") { | ||
| return {}; |
There was a problem hiding this comment.
Fine if Redis cache removal isn't switched on, but if it is, is this an error?
There was a problem hiding this comment.
This is specifically for the landing page of the journey where we have no transactionID and no submissionID, so it shouldn't error but should return an empty object.
| return {}; | ||
| } | ||
|
|
||
| const appData = await getOverseasEntity(req, transactionId, submissionId); |
There was a problem hiding this comment.
Are tests around this changed functionality to follow later?
There was a problem hiding this comment.
Yes, these were to follow later, but the revised implementation means they will now be part of a future PR.
| try { | ||
|
|
||
| logger.debugRequest(req, `POST ${config.SOLD_LAND_FILTER_PAGE}`); | ||
|
|
There was a problem hiding this comment.
Can't see any tests for this changed functionality.
| if (hasSoldLand !== '0') { | ||
| nextPageUrl = config.CANNOT_USE_URL; | ||
| } else { | ||
| if (!appData[Transactionkey]) { |
There was a problem hiding this comment.
I was expecting this change in behaviour to be feature flagged but it's not. That might be ok, but we need to discuss. Relates to this comment on the API change:
https://github.com/companieshouse/overseas-entities-api/pull/404/files#r1673848116
There was a problem hiding this comment.
To me, it makes sense to put this behind the Redis feature flag as it's a fundamental change to the behaviour.
There was a problem hiding this comment.
Agreed. Feature flag added to new PR here.
mwejuli-ch
changed the title
sold-land-filter option to databasesold-land-filter option to database
mwejuli-ch
changed the title
sold-land-filter option to databasesold-land-filter option to database
mwejuli-ch
changed the title
sold-land-filter option to databasesold-land-filter option to database
JIRA link
https://companieshouse.atlassian.net/browse/ROE-2559
Change description
sold-land-filteroption to the APIgetApplicationDatamethod to fetch data from APIWork checklist