Skip to content
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

Replaced useDispatch with useQuery and request in ExternalResult (src/Components/ExternalResult/** ) #6402

Merged
merged 13 commits into from
Oct 17, 2023

Conversation

konavivekramakrishna
Copy link
Contributor

@konavivekramakrishna konavivekramakrishna commented Oct 5, 2023

WHAT

🤖 Generated by Copilot at de8a84f

This pull request refactors and simplifies the code for the external result feature of the application. It uses the useQuery hook and the request utility to fetch and update data from the API, and defines the types and interfaces for the external result data and API requests and responses in the models.ts file. It also fixes some TypeScript and React errors and warnings, and removes some unused actions from the actions.tsx file.

Proposed Changes

@coronasafe/care-fe-code-reviewers @coronasafe/code-reviewers

Merge Checklist

  • Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

HOW

🤖 Generated by Copilot at de8a84f

  • Refactor the external result components to use the useQuery hook and the request utility for API calls and response handling (link, link, link, link, link, link, link, link)
  • Remove the unused useDispatch and useAbortableEffect hooks from the external result components (link, link, link, link, link)
  • Remove the unused actions from the actions.tsx file (link, link)
  • Add the models.ts file to define the interfaces and types for the external result data and API requests and responses (link)
  • Import the models.ts file and add the method, TRes, and TBody properties to the external result endpoints in the api.tsx file (link, link, link, link, link)
  • Rename the data variable to resultItemData in the ResultItem component to avoid confusion with the data variable returned by the useQuery hook (link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Rename the data variable to resultListData in the ResultList component to avoid confusion with the data variable returned by the useQuery hook (link, link, link)
  • Rename the data variable to rdata in the ResultUpdate component to avoid confusion with the data variable returned by the request utility (link)
  • Remove the unused imports from the ExternalResultUpload component (link)
  • Add the useQuery, routes, and Loading modules to the imports in the ListFilter component (link)
  • Remove the dispatch variable from the ListFilter component (link)
  • Remove the useEffect hook that was used to fetch the ward and local body lists based on the district from the ListFilter component (link)
  • Add the loading variable to conditionally render the Loading component in the ListFilter and ResultUpdate components (link, link)
  • Add the useQuery, routes, and request modules to the imports in the ResultItem and ResultUpdate components (link, link)
  • Check the showDeleteAlert state before calling the handleDelete function in the ResultItem component (link)
  • Add the loading variable to the dependency array of the useEffect hook in the ResultList component (link)
  • Wrap the result.name variable in a template literal to avoid a TypeScript error in the ResultList component (link)
  • Add the key prop to the input element to avoid a React warning in the ResultList component (link)
  • Modify the fetchWards function to take a number as an argument instead of a string in the ResultUpdate component (link)

@vercel
Copy link

vercel bot commented Oct 5, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
care-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 11, 2023 5:06am

@netlify
Copy link

netlify bot commented Oct 5, 2023

Deploy Preview for care-egov-staging ready!

Name Link
🔨 Latest commit 7694cb3
🔍 Latest deploy log https://app.netlify.com/sites/care-egov-staging/deploys/65262d4768bef700086633c8
😎 Deploy Preview https://deploy-preview-6402--care-egov-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@konavivekramakrishna
Copy link
Contributor Author

Hey @rithviknishad
delete record is not working as expected and I am still working on fixing it
Apart from that are there any changes required?

const [data, setData] = useState([]);
const [resultListData, setResultListData] = useState<
Partial<IExternalResult>[]
>([]);
const [isLoading, setIsLoading] = useState(false);
const [totalCount, setTotalCount] = useState(0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need a state here again.

Suggested change
const [totalCount, setTotalCount] = useState(0);
const totalCount = data ? data.count : 0

Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@konavivekramakrishna the delete being fetched two times is because, the request is ending up performing a reattempt because it failed to parse the JSON on first attempt.

Although it was a success response on first attempt, it attempted to parse the empty response body as JSON and hence caused to reattempt.

You can update the request.ts to only parse the JSON if the response has content type header as JSON.

@konavivekramakrishna
Copy link
Contributor Author

konavivekramakrishna commented Oct 6, 2023

@rithviknishad
when trying to implement PaginatedListin ResultList.tsx, filtering is not working. Can you please review it once? Should I make a PR, or should I continue working on implementing PaginatedList?

@konavivekramakrishna konavivekramakrishna changed the title Fix#6360 Replaced useDispatch with useQuery and request in ExternalResult (src/Components/ExternalResult/ Oct 6, 2023
@konavivekramakrishna konavivekramakrishna changed the title Replaced useDispatch with useQuery and request in ExternalResult (src/Components/ExternalResult/ Replaced useDispatch with useQuery and request in ExternalResult (src/Components/ExternalResult/** ) Oct 6, 2023
@konavivekramakrishna konavivekramakrishna marked this pull request as ready for review October 6, 2023 15:48
@konavivekramakrishna konavivekramakrishna requested a review from a team October 6, 2023 15:48
@konavivekramakrishna konavivekramakrishna requested a review from a team as a code owner October 6, 2023 15:48
@netlify
Copy link

netlify bot commented Oct 9, 2023

Deploy Preview for care-net failed.

Name Link
🔨 Latest commit 29fec55
🔍 Latest deploy log https://app.netlify.com/sites/care-net/deploys/6523fafec6d37600081bc84b

@nihal467
Copy link
Member

@rithviknishad can you review this

@rithviknishad rithviknishad removed work-in-progress Deploy-Failed Deplyment is not showing preview labels Oct 11, 2023
Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! LGTM

@nihal467
Copy link
Member

LGTM

@khavinshankar khavinshankar merged commit c776524 into coronasafe:develop Oct 17, 2023
31 of 33 checks passed
rithviknishad added a commit that referenced this pull request Oct 17, 2023
…/Components/ExternalResult/** ) (#6402)

* replaced dispatch with useQuery in ResultList

* replaced useDispatch with useQuery in listfilter

* replaced useDispatch with reqeust in ExternalResultUpload

* fixed filtering

* fix ResultList

* rm clg

* make code readable

* fix build

* removed useEffect and useState from ResultList.tsx

* Apply changes from code review

---------

Co-authored-by: rithviknishad <mail@rithviknishad.dev>
Ashesh3 pushed a commit that referenced this pull request Oct 20, 2023
…/Components/ExternalResult/** ) (#6402)

* replaced dispatch with useQuery in ResultList

* replaced useDispatch with useQuery in listfilter

* replaced useDispatch with reqeust in ExternalResultUpload

* fixed filtering

* fix ResultList

* rm clg

* make code readable

* fix build

* removed useEffect and useState from ResultList.tsx

* Apply changes from code review

---------

Co-authored-by: rithviknishad <mail@rithviknishad.dev>
Ashesh3 pushed a commit that referenced this pull request Oct 23, 2023
…/Components/ExternalResult/** ) (#6402)

* replaced dispatch with useQuery in ResultList

* replaced useDispatch with useQuery in listfilter

* replaced useDispatch with reqeust in ExternalResultUpload

* fixed filtering

* fix ResultList

* rm clg

* make code readable

* fix build

* removed useEffect and useState from ResultList.tsx

* Apply changes from code review

---------

Co-authored-by: rithviknishad <mail@rithviknishad.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🛠️ Replace useDispatch w. useQuery/request: ExternalResult (src/Components/ExternalResult/**)
4 participants