OBPIH-7555 Create UI for Expiration History report table (fixes)#5602
OBPIH-7555 Create UI for Expiration History report table (fixes)#5602alannadolny merged 7 commits intodevelopfrom
Conversation
SebastianLib
requested review from
alannadolny,
awalkowiak,
ewaterman and
kchelstowski
github-actions
bot
added
domain: frontend
| background-color: var(--page-background); | ||
| display: flex; | ||
| padding: 0.5rem; | ||
| background-color: var(--blue-100); | ||
|
|
||
| div { | ||
| background-color: var(--blue-100); | ||
| } | ||
| } | ||
|
|
||
| .table-pagination-bottom { | ||
| background-color: var(--blue-100) !important; |
There was a problem hiding this comment.
Before my changes the footer with total amount looked like this
now it looks like in the old tables:
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #5602 +/- ##
============================================
- Coverage 9.12% 8.52% -0.61%
+ Complexity 1170 1118 -52
============================================
Files 701 712 +11
Lines 45281 45598 +317
Branches 10851 10913 +62
============================================
- Hits 4131 3885 -246
- Misses 40497 41135 +638
+ Partials 653 578 -75 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
SebastianLib
force-pushed
the
fix/OBPIH-7555
branch
from
86e3345 to
99912fd
Compare
SebastianLib
force-pushed
the
fix/OBPIH-7555
branch
from
99912fd to
b46c0ae
Compare
| Map<String, Number> getExpirationHistoryReportTotals(List<ExpirationHistoryReportRow> entries) { | ||
| Integer totalQuantityLostToExpiry = entries.sum { it?.quantityLostToExpiry ?: 0 } as Integer ?: 0 | ||
| BigDecimal totalValueLostToExpiry = entries.sum { it?.valueLostToExpiry ?: 0 } as BigDecimal ?: 0 | ||
|
|
||
| return [ | ||
| totalQuantityLostToExpiry: totalQuantityLostToExpiry, | ||
| totalValueLostToExpiry : totalValueLostToExpiry | ||
| ] as Map<String, Number> | ||
| } |
There was a problem hiding this comment.
@kchelstowski I hope this is done as you wanted 🤔
There was a problem hiding this comment.
This is somewhat of a nitpick but since this logic is quite tied to the getExpirationHistoryReport logic it might make more sense to put them together in a single object:
class ExpirationHistoryReport {
List<ExpirationHistoryReportRow> rows
Integer totalQuantityLostToExpiry
BigDecimal totalValueLostToExpiry
}
and return that in getExpirationHistoryReport. As a bonus it saves you from needing to work with a map (with non-concrete keys) for the totals
There was a problem hiding this comment.
Oh, sorry but I didn’t see your comment. I think that sounds good, so I can try to do something like this tomorrow. @kchelstowski what do you think about this idea?
There was a problem hiding this comment.
I had that in mind when thinking about this for the first time, but I wanted to keep it as simple as I could for Sebastian. If you feel confident @SebastianLib you can go with that. Remember though that the rows that Evan mentioned are supposed to be PaginatedList, not List.
| filtersInitialized, | ||
| }); | ||
|
|
||
| const totalAmount = () => `${translate('react.report.expirationHistory.totalAmount.label', 'Total amount')}: ${tableData?.totalValueLostToExpiry?.toLocaleString([LocaleConverter[locale] || Locale.EN]) ?? 0} ${currencyCode}`; |
There was a problem hiding this comment.
can't it be just a const, not a method?
There was a problem hiding this comment.
Good catch, I didn’t even notice this because I copied the totalAmount function from the Purchase Order List, because we have similar logic there
| // Update the reference to the current location for future checks | ||
| previousLocation.current = currentLocation?.id; | ||
| } | ||
| }, [currentLocation?.id]); |
There was a problem hiding this comment.
this can potentially become a reusable hook that would accept a callback argument - in this case the setTableData(...) and in the other file, it would accept the setFilterParams(..).
@alannadolny thoughts?
| return new PaginatedList<ExpirationHistoryReportRow>(entries.collect { ExpirationHistoryReportRow.fromTransactionEntry(it) }, entries.totalCount) | ||
|
|
||
| List<ExpirationHistoryReportRow> rows = entries.collect { ExpirationHistoryReportRow.fromTransactionEntry(it) } | ||
| PaginatedList<ExpirationHistoryReportRow> paginatedRows = new PaginatedList<ExpirationHistoryReportRow>(rows, entries.totalCount) |
There was a problem hiding this comment.
nitpicky: I don't see a point in separating those two. I think you can just copy the 3476th line and just have rows, and then, in the calculations above just refer to rows.list
| import useSpinner from 'hooks/useSpinner'; | ||
| import { transformFilterParams } from 'utils/list-utils'; | ||
|
|
||
| import useOnLocationChange from '../useOnLocationChange'; |
4 participants
✨ Description of Change
Link to GitHub issue or Jira ticket:
https://pihemr.atlassian.net/browse/OBPIH-7555
Description:
📷 Screenshots & Recordings (optional)