Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

refactor(edit patient): use react-query when editing a patient #2466

Closed
wants to merge 5 commits into from
Closed

refactor(edit patient): use react-query when editing a patient #2466

wants to merge 5 commits into from

Conversation

giulianovarriale
Copy link
Contributor

@giulianovarriale giulianovarriale commented Oct 31, 2020 •
edited
Loading

Fixes #2382.

Changes proposed in this pull request:

  • Use react-query for updating a new patient
  • Remove redux code that was used for updating a new patient

@jsf-clabot
Copy link

jsf-clabot commented Oct 31, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@gitpod-io
Copy link

gitpod-io bot commented Oct 31, 2020 •
edited
Loading

@vercel
Copy link

vercel bot commented Oct 31, 2020 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/hospitalrun/hospitalrun-frontend/m8zaihnnj
✅ Preview: https://hospitalrun-fr-git-fork-giulianovarriale-refactor-update-9a186c.hospitalrun.vercel.app

@giulianovarriale giulianovarriale marked this pull request as ready for review October 31, 2020 21:22
@giulianovarriale
Copy link
Contributor Author

giulianovarriale commented Oct 31, 2020 •
edited
Loading

The PR is not finished yet, I opened it already for two reasons:

  1. I'm participating in hacktoberfest and today is the last day to contribute.
    Anyway, I'll finish my contribution because this is the right thing to do and also I'm having fun implementing react-query on the project.
  2. I'm wondering what you expect to be tested in here, since the feature remains the same, but the implementation has been changed. For this PR, I was thinking to write a test for the validator and also a test for the hook, what do you think?

Also, the validation is being shared between the create and edit actions. I think I can create a new PR for the patient validation and then after is merged, I can use it her and also on #2467. Should I go for it?

@matteovivona matteovivona changed the title refactor(edit patient): use react-query when editing a patient WIP: refactor(edit patient): use react-query when editing a patient Oct 31, 2020
@matteovivona matteovivona added hacktoberfest-accepted in progress indicates that issue/pull request is currently being worked on labels Oct 31, 2020
jackcmeyer
jackcmeyer suggested changes Nov 1, 2020
View reviewed changes
src/patients/edit/EditPatient.tsx Outdated Show resolved Hide resolved
src/patients/util/validate-patient.ts Outdated Show resolved Hide resolved
src/patients/hooks/useUpdatePatient.tsx
return PatientRepository.saveOrUpdate(cleanPatient)
}

export default function useUpdatePatient() {
Copy link
Member

Choose a reason for hiding this comment

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

upon success of the mutation, in should update the patient in the cache.

src/patients/edit/EditPatient.tsx Outdated Show resolved Hide resolved
src/patients/util/validate-patient.ts Outdated Show resolved Hide resolved
@jackcmeyer
Copy link
Member

For this PR, I was thinking to write a test for the validator and also a test for the hook, what do you think?

I think this sounds perfect.

@giulianovarriale
Copy link
Contributor Author

@jackcmeyer TBH, I didn't write the validator code. I just copied and pasted from the patient-slice file. I think the validator should be out of the scope of this PR. What I'm gonna do is to open a new PR for the validator and once that one is merged we can rebase this one and use it in here. Do you agree?

@giulianovarriale
Copy link
Contributor Author

@jackcmeyer Please, before moving on with this PR, take a look on this one: #2471

@matteovivona matteovivona changed the title WIP: refactor(edit patient): use react-query when editing a patient refactor(edit patient): use react-query when editing a patient Nov 6, 2020
@matteovivona matteovivona added this to In progress in Version 2.0 via automation Nov 6, 2020
@matteovivona matteovivona added this to the v2.0 milestone Nov 6, 2020
@matteovivona matteovivona self-requested a review November 6, 2020 20:42
matteovivona
matteovivona previously approved these changes Nov 6, 2020
View reviewed changes
@matteovivona
Copy link
Contributor

@giulianovarriale can you resolve the conflict? thanks

@giulianovarriale
Copy link
Contributor Author

I did that already on my local environment. I'm writing the hook test. Once I finished it, I'll push it.

@giulianovarriale
Copy link
Contributor Author

@tehkapa It is ready for review again.

Version 2.0 automation moved this from In progress to Done Jun 1, 2021
@giulianovarriale giulianovarriale deleted the refactor/update-patient branch June 1, 2021 21:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@matteovivona matteovivona matteovivona approved these changes

@fox1t fox1t Awaiting requested review from fox1t

@jackcmeyer jackcmeyer Awaiting requested review from jackcmeyer

Assignees
No one assigned
Labels
hacktoberfest-accepted in progress indicates that issue/pull request is currently being worked on
Projects
Version 2.0
  
Done
Milestone
v2.0
Development

Successfully merging this pull request may close these issues.

create useUpdatePatient hook and remove related redux code
4 participants
@giulianovarriale @jsf-clabot @jackcmeyer @matteovivona