PIC-4093: Code change to display matching defendant records in descen… #990
Conversation
server/routes/helpers.js
Outdated
| const getPaginationObject = (pageParams) => { | ||
| const maximumPages = 4 | ||
| const currentPage = pageParams.page | ||
| let startNum = pageParams.page - ((maximumPages - 1) / 2) |
There was a problem hiding this comment.
Might as well use currentPage here instead of pageParams.page
Might even be nice to destructure pageParams into something like
({page: currentPage, limit,matchingRecordsCount})
server/routes/helpers.js
Outdated
| let endNum = pageParams.page + ((maximumPages - 1) / 2) | ||
| const totalPages = Math.round(Math.ceil((pageParams.matchingRecordsCount / pageParams.limit))) | ||
|
|
||
| const startCount = ((parseInt(pageParams.page, 10) - 1) || 0) * pageParams.limit |
There was a problem hiding this comment.
is the parseInt doing anything useful here? Compare line 65 where we subtract from it without parseInt first.
server/routes/index.js
Outdated
| const currentPage = parseInt(queryParams.page, 10) || 1 | ||
| const limit = 5 | ||
|
|
||
| // Calculate start and end index for slicing data |
There was a problem hiding this comment.
Maybe this logic belongs in getPaginationObject? It seems to be derived from values we are passing into that method anyway?
| @@ -27,6 +27,11 @@ | |||
|
|
|||
| .pac-\!-grid-column-45 { | |||
| width: 45%; | |||
| padding: 10px 0; | |||
There was a problem hiding this comment.
I'm worried that putting this here with mean if someone uses this in the future we have these overrides.... Could we add it as an additional class ?
There was a problem hiding this comment.
Once we have the handler in a new routing file - we need to make sure we have unit tests for it too. I can help you with these. We either follow a given, when, then style or I use arrange, act, assert which is much the same.
server/routes/index.js
Outdated
| const response = await getMatchDetails(defendantId) | ||
|
|
||
| const matchingRecordsCount = response.offenderMatchDetails.length |
There was a problem hiding this comment.
Do we ever check if this is zero? I know that to get here there must be a match as it's from a list but just wondering as it's a wild edge case.
There was a problem hiding this comment.
Also as we're not typescript can we checkresponse.offenderMatchDetails is an array...
server/services/case-service.js
Outdated
| @@ -34,6 +34,14 @@ const createCaseService = apiUrl => { | |||
| const res = (await request( | |||
| `${apiUrl}/defendant/${defendantId}/matchesDetail` | |||
| )) || { data: {} } | |||
| // Sort offenderMatchDetails based on matchProbability in descending order | |||
| if (res.data.offenderMatchDetails) { | |||
There was a problem hiding this comment.
Do we want to check this is an array? just in case we get nothing?
server/routes/index.js
Outdated
| const response = await getMatchDetails(defendantId) | ||
|
|
||
| const matchingRecordsCount = response.offenderMatchDetails.length | ||
| const currentPage = parseInt(queryParams.page, 10) || 1 |
There was a problem hiding this comment.
This might be a bit over the top but what if someone changes the url and adds a minus or not a number?
server/routes/index.js
Outdated
|
|
||
| const matchingRecordsCount = response.offenderMatchDetails.length | ||
| const currentPage = parseInt(queryParams.page, 10) || 1 | ||
| const limit = 5 |
server/routes/index.js
Outdated
| if (!Array.isArray(offenderMatchDetails)) { | ||
| return res.render('error', { | ||
| ...templateValues, | ||
| message: 'Unexpected data format received from the server.' |
There was a problem hiding this comment.
Is there a test for this?
Bumps [webpack](https://github.com/webpack/webpack) from 5.89.0 to 5.94.0. - [Release notes](https://github.com/webpack/webpack/releases) - [Commits](webpack/webpack@v5.89.0...v5.94.0) --- updated-dependencies: - dependency-name: webpack dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
build(deps-dev): bump webpack from 5.89.0 to 5.94.0 (#980) Bumps [webpack](https://github.com/webpack/webpack) from 5.89.0 to 5.94.0. - [Release notes](https://github.com/webpack/webpack/releases) - [Commits](webpack/webpack@v5.89.0...v5.94.0) --- updated-dependencies: - dependency-name: webpack dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> PIC-4093: Changes based on review PIC-4093: Code change to display matching defendant records in descending order.
…ding order.
JIRA Link : https://dsdmoj.atlassian.net/browse/PIC-4092
JIRA Link : https://dsdmoj.atlassian.net/browse/PIC-4093