NNS1-3486: period filter for reporting transactions #6032
Conversation
yhabib
force-pushed
the
NNS1-3486/clean-up-types
branch
from
cbccad8 to
06fb4af
Compare
yhabib
force-pushed
the
NNS1-3486/filter-transactions
branch
2 times, most recently
from
a07715b to
276db08
Compare
yhabib
force-pushed
the
NNS1-3486/filter-transactions
branch
from
276db08 to
0b6204f
Compare
| @@ -5,7 +5,7 @@ import { render } from "@testing-library/svelte"; | |||
|
|
|||
| describe("ReportingDateRangeSelector", () => { | |||
| const renderComponent = () => { | |||
| const { container } = render(ReportingDateRangeSelector); | |||
| const { container } = render(ReportingDateRangeSelector, { period: "all" }); | |||
There was a problem hiding this comment.
Please add a unit test that selecting an option changes the exported prop.
And a unit test that changing the prop changes the selected option.
| @@ -123,7 +123,7 @@ | |||
| label: $i18n.reporting.timestamp, | |||
| }, | |||
| ]; | |||
| const fileName = `icp_transactions_export_${formatDateCompact(new Date())}`; | |||
| const fileName = `icp_transactions_export_${period}_${formatDateCompact(new Date())}`; | |||
There was a problem hiding this comment.
If it's "last year", does it still make sense to put the current date in the filename, rather than last year's year?
There was a problem hiding this comment.
I will revert the change and sync with the PO to decide how to proceed.
There was a problem hiding this comment.
Not for this PR, but the static under functions for page objects don't need named parameters if there's only a single parameter. We only do that if an additional testId needs to be passed.
|
|
||
| await tick(); | ||
|
|
||
| const currentValue = component.$$.ctx[component.$$.props["period"]]; |
There was a problem hiding this comment.
This is how I check if the value is updated. Is there a better way?
There was a problem hiding this comment.
This is fine. Another possibility is to add a custom component for testing, which exposes the value in the DOM.
yhabib
force-pushed
the
NNS1-3486/filter-transactions
branch
from
1d72c17 to
581b2a0
Compare
|
|
||
| await tick(); | ||
|
|
||
| const currentValue = component.$$.ctx[component.$$.props["period"]]; |
There was a problem hiding this comment.
This is fine. Another possibility is to add a custom component for testing, which exposes the value in the DOM.
| await tick(); | ||
|
|
||
| const currentValue = component.$$.ctx[component.$$.props["period"]]; | ||
| expect(currentValue).toBe("last-year"); |
There was a problem hiding this comment.
Please also expect a different value before clicking on the option.
I recommend moving the code to get the prop value into a function so the test itself is more readable.
yhabib
force-pushed
the
NNS1-3486/filter-transactions
branch
from
eb53247 to
34b503e
Compare
Motivation
We want users to be able to generate three types of reports:
https://qsgjb-riaaa-aaaaa-aaaga-cai.yhabib-ingress.devenv.dfinity.network/reporting/
Changes
periodstate toReportingTransactions. It binds it toReportingDateRangeSelectorso it can be updated, and it passes it down toReportingTransactionsButtonto be used when generating the report.Tests
ReportingTransactionsTodos