Feature/filter table #129
Feature/filter table #129
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
Thank you for raising the request! RAD Lab admins have been notified. |
|
Apache 2.0 License check successful! |
acolver
left a comment
There was a problem hiding this comment.
Can you please write some unit tests with several mock deployments that test the search, filtering, and sorting functionality? I see tests that check if the components are rendering, but we also want tests that check the functionality.
| expect(linkElement2).toBeInTheDocument() | ||
| expect(linkElement2).toBeEnabled() | ||
| const allDeployments = screen.getByTestId("all-deployments") | ||
| isAdmin && expect(allDeployments).toBeInTheDocument() |
There was a problem hiding this comment.
why have isAdmin here? It's always true.
| it("should show all deployment lists", async () => { | ||
| render( | ||
| <BrowserRouter> | ||
| <ModuleDeployment | ||
| headers={DEPLOYMENT_HEADERS} | ||
| deployments={deploymentMockData} | ||
| defaultSortField={SORT_FIELD.CREATEDAT} | ||
| defaultSortDirection={SORT_DIRECTION.DESC} | ||
| /> | ||
| </BrowserRouter>, | ||
| ) | ||
| }) |
There was a problem hiding this comment.
There are no assertions here
| </div> | ||
| {deploymentTab === DEPLOYMENT_TAB.ALL && ( | ||
| <> | ||
| {listAllDeployments?.length ? ( |
There was a problem hiding this comment.
You don't need this first check
There was a problem hiding this comment.
renderEmptySearch() is a function which will be used when we search and find that no data is present according to search and renderEmptyState() is used when there is no deployment.
There was a problem hiding this comment.
Should I write some other logic to handle the condition?
| )} | ||
| {deploymentTab === DEPLOYMENT_TAB.MINE && ( | ||
| <> | ||
| {listMyDeployments?.length ? ( |
There was a problem hiding this comment.
You don't need this first check
No description provided.