feat(Bonus Pagamenti Digitali): [#175741693] Remove timestamp from transactions when it's equal to 00:00 #2734
Conversation
…nt and added related tests
GiovanniMancini
requested review from
CrisTofani,
fabriziofff and
Undermaken
as code owners
pagopa-github-bot
changed the title
Affected stories
|
Codecov Report
@@ Coverage Diff @@
## master #2734 +/- ##
==========================================
+ Coverage 53.36% 53.45% +0.09%
==========================================
Files 760 760
Lines 21216 21224 +8
Branches 3697 4032 +335
==========================================
+ Hits 11322 11346 +24
+ Misses 9841 9823 -18
- Partials 53 55 +2
Continue to review full report at Codecov.
|
| @@ -0,0 +1,56 @@ | |||
| // import * as React from "react"; | |||
There was a problem hiding this comment.
Could these imports be removed?
There was a problem hiding this comment.
Sure.. I'll clean useless comments
| describe("Test how the transaction subtitle changes with different timestamps", () => { | ||
| it("Subtitle shouldn't contain hours and minutes, when the transaction has a timestamp 00:00", () => { | ||
|
|
||
| // TODO: test input data should be moved in a separate file |
There was a problem hiding this comment.
TODO should be followed by a ref to a story (es
) or Can we remove it?There was a problem hiding this comment.
I'll remove the TODO.. I don't know if the team agrees on having a separate module for test input data, this is just my approach
| <H4 weight={"SemiBold"} color={"bluegreyDark"}> | ||
| {localeDateFormat( | ||
| <H4 weight={"SemiBold"} color={"bluegreyDark"} testID="dateValue"> | ||
| { (props.transaction.trxDate.getHours() === 0 && props.transaction.trxDate.getMinutes() === 0) ? |
There was a problem hiding this comment.
as improvement, could it be better to calculate a const once instead of make the same op twice?
const isMidNight =
props.transaction.trxDate.getHours() +
props.transaction.trxDate.getMinutes() ===
0;There was a problem hiding this comment.
Thanks, your solution is more clear and readable. I'll change as you suggest
…pagopa/io-app into 175741693-transaction-timestamp
| )} · € ${formatNumberAmount(transaction.amount)} `; | ||
| export const getSubtitle = (transaction: BpdTransaction) => { | ||
| const isMidNight = | ||
| transaction.trxDate.getHours() + transaction.trxDate.getMinutes() === 0; |
There was a problem hiding this comment.
What you think about adding also the seconds to the calculation?
| transaction.trxDate.getHours() + transaction.trxDate.getMinutes() === 0; | |
| transaction.trxDate.getHours() + transaction.trxDate.getMinutes() + transaction.trxDate.getSeconds() === 0; |
In this way we can render the time for a transaction with timestamp 2020-12-13T00:00:31+01:00 that is a valid transaction.
| const isMidNight = | ||
| props.transaction.trxDate.getHours() + | ||
| props.transaction.trxDate.getMinutes() === | ||
| 0; |
There was a problem hiding this comment.
The same consideration about considering the seconds of the previous comment.
…pagopa/io-app into 175741693-transaction-timestamp
Short description
This PR proposes to hide the timestamp of a transaction when it's equal to
00:00as in this case it isn't reliable.List of changes proposed in this pull request
getSubtitleinBdpTransactionItemto conditionally format subtitle of transactionsBpdTransactionDetailComponentHow to test
Unit tests: I've added 2 suites of unit tests.
Visual tests: see attached images
With amount before date
With date before amount