-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENT-7820 Learner Credit table sorting #373
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running the PR locally, it is sorting as expected 👍🏽 but noticed some failing tests. Will run the CI on this for now while it gets corrected.
renderWithRouter(<DashboardDatatableWrapper />); | ||
const tableHeader = screen.getByText('Start date'); | ||
|
||
fireEvent.click(tableHeader); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of fireEvent
the repo is leaning towards userEvent
in testing. See
Lines 69 to 90 in f00e83f
it('renders policy type and selects AssignedLearnerCreditAccessPolicy', async () => { | |
const updatedInitialState = { | |
...initialStateValue, | |
multipleFunds: false, | |
formData: { | |
...initialStateValue.formData, | |
policies: INITIAL_POLICIES.singlePolicy, | |
}, | |
}; | |
renderWithRouter( | |
<ProvisioningFormPolicyContainerWrapper | |
value={updatedInitialState} | |
sampleCatalogQuery={INITIAL_POLICIES.singlePolicy} | |
/>, | |
); | |
expect(screen.getByText(POLICY_TYPE.TITLE)).toBeTruthy(); | |
expect(screen.getByText(POLICY_TYPE.LABEL)).toBeTruthy(); | |
userEvent.click(screen.getByTestId(POLICY_TYPE.OPTIONS.ADMIN_SELECTS.DESCRIPTION)); | |
await waitFor(() => { | |
expect(screen.getByTestId(POLICY_TYPE.OPTIONS.ADMIN_SELECTS.DESCRIPTION).checked).toBeTruthy(); | |
}); | |
}); |
import userEvent from '@testing-library/user-event';
const tableHeader = screen.getByText('Start date');
userEvent.click(tableHeader);
if (sortByObject.id === 'isActive') { | ||
return sortByObject.desc ? '-expirationDatetime' : 'expirationDatetime'; | ||
let sortString = sortByObject.id; | ||
if ( sortString === 'isActive') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback. Sorted the linting issues out and using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏽 Great job
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #373 +/- ##
==========================================
+ Coverage 88.09% 88.10% +0.01%
==========================================
Files 162 162
Lines 3377 3380 +3
Branches 837 836 -1
==========================================
+ Hits 2975 2978 +3
Misses 398 398
Partials 4 4 ☔ View full report in Codecov by Sentry. |
There were a couple issues with the
sortDataTableData
method.lodash.snakecase
was stripping out the-
which is used for sorting in reverse. Moved thesnakecase
call before the-
was prepended.Added some test for asc and desc sorting.