feat: Update files explorer tab with components selector #2450
Conversation
![codecov[bot]](https://avatars.githubusercontent.com/in/254?s=60&v=4){kind=small}
There was a problem hiding this comment.
The proposed changes seem to be fine overall. In these changes, a new component called 'ComponentsSelector' is added to the 'FileExplorer'. It also updates the related files and test cases. However, a couple of improvement points are to be considered to ensure code integrity, clearness, for better validation and error handling.
| searchValue={params?.search} | ||
| setSearchValue={(search) => updateParams({ search })} | ||
| /> | ||
| <div className="flex gap-2"> |
There was a problem hiding this comment.
You have added a new component (ComponentsSelector) but did not provide any prop. Please make sure to add necessary props and validate them using propTypes in the 'ComponentsSelector' component. This will increase type safety and avoid possible issues in the future.
| describe('when rendered', () => { | ||
| it('renders ComponentsSelector', async () => { | ||
| setup() | ||
| render(<FileExplorer />, { wrapper: wrapper() }) |
There was a problem hiding this comment.
Ensure that you have handled potential exceptions and erroneous cases by writing negative test cases as well, not just happy path scenarios. This will greatly boost the confidence in the correctness of the code.
| @@ -153,7 +153,7 @@ const sortingParameter = Object.freeze({ | |||
| lines: 'LINES', | |||
| }) | |||
|
|
|||
| const getQueryFilters = ({ params, sortBy, flags }) => { | |||
| const getQueryFilters = ({ params, sortBy, flags, components }) => { | |||
There was a problem hiding this comment.
Check that the 'components' parameter is incorporated correctly into the 'getQueryFilters' function.
| @@ -177,6 +178,8 @@ export function useRepoPullContentsTable() { | |||
| depth: 1, | |||
| }) | |||
There was a problem hiding this comment.
Query parameters should be validated before use. Improper or lack of validation could lead to potential security vulnerabilities such as Injections, and Denial of Service(DoS) if the input is not what's expected.
| repo: 'test-repo', | ||
| path: '', | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Please check if you have any memory leaks in your hooks especially those containing listeners or subscriptions.
✅ Deploy Preview for gazebo-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -13,6 +13,8 @@ import Spinner from 'ui/Spinner' | |||
|
|
|||
| import { useRepoPullContentsTable } from './hooks' | |||
|
|
|||
| import ComponentsSelector from '../ComponentsSelector' | |||
There was a problem hiding this comment.
Ensure that 'ComponentsSelector' is properly exported from its module, as it is being imported here.
| @@ -187,7 +190,7 @@ export function useRepoPullContentsTable() { | |||
| repo, | |||
There was a problem hiding this comment.
Update the code according to the new data structure introduced on line 75, considering the 'components' filter as well.
| @@ -258,6 +258,63 @@ describe('useRepoPullContentsTable', () => { | |||
| }) | |||
| }) | |||
|
|
|||
| describe('when called with a selected components', () => { | |||
| it('makes a gql request with the list param', async () => { | |||
There was a problem hiding this comment.
Validate that new unit tests are working properly that handle additional parameter 'components'.
| }) | ||
| }) | ||
|
|
||
| describe('when called with a selected flags and components', () => { |
There was a problem hiding this comment.
Validate that new unit tests are working properly that handle both parameters 'flags' and 'components'.
| placeholder="Search for files" | ||
| searchValue={params?.search} | ||
| setSearchValue={(search) => updateParams({ search })} | ||
| /> |
There was a problem hiding this comment.
Keep an eye on CSS to make sure that the combination of the 'ComponentsSelector' and the 'SearchField' within the flex container looks good in all main browsers.
| @@ -194,6 +196,16 @@ describe('FileExplorer', () => { | |||
| }) | |||
| }) | |||
|
|
|||
There was a problem hiding this comment.
Use a more explicit test for 'renders ComponentsSelector' to ensure that it contributes to the component and its behaviors as expected.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2450 +/- ##
=======================================
Coverage 98.07% 98.07%
=======================================
Files 763 763
Lines 9699 9701 +2
Branches 2443 2444 +1
=======================================
+ Hits 9512 9514 +2
Misses 185 185
Partials 2 2
Continue to review full report in Codecov by Sentry.
|
Codecov Report
@@ Coverage Diff @@
## main #2450 +/- ##
=======================================
Coverage 98.07% 98.07%
=======================================
Files 763 763
Lines 9699 9701 +2
Branches 2448 2444 -4
=======================================
+ Hits 9512 9514 +2
Misses 185 185
Partials 2 2
Continue to review full report in Codecov by Sentry.
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2450 +/- ##
=====================================
Coverage 98.07 98.07
=====================================
Files 763 763
Lines 9699 9701 +2
Branches 2448 2444 -4
=====================================
+ Hits 9512 9514 +2
Misses 185 185
Partials 2 2
Continue to review full report in Codecov by Sentry.
|
Description
Adding the selector to PR files explorer
Notable Changes
Screenshots
Screen.Recording.2023-12-12.at.4.00.13.PM.mov
Link to Sample Entry
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.