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.

Sign up for GitHub

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

Jump to bottom

Elation find physician #109

Merged
merged 15 commits into from
Jun 30, 2023
Merged

Elation find physician #109

merged 15 commits into from
Jun 30, 2023

Conversation

michal-grzelak
Copy link
Contributor

@michal-grzelak michal-grzelak self-assigned this Apr 26, 2023
bejoinka
bejoinka requested changes Apr 28, 2023
View reviewed changes
Copy link
Contributor

@bejoinka bejoinka left a comment

Choose a reason for hiding this comment

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

looks good, almost there. looks like you missed the query strings. fields should be first_name, last_name, and npi. they should be passed as query params in the get request to elation

@michal-grzelak
Copy link
Contributor Author

@bejoinka

looks good, almost there. looks like you missed the query strings. fields should be first_name, last_name, and npi. they should be passed as query params in the get request to elation

Thanks for spotting this. I totally missed those 🙈

bejoinka
bejoinka requested changes May 1, 2023
View reviewed changes
Copy link
Contributor

@bejoinka bejoinka left a comment •
edited by michal-grzelak
Loading

Choose a reason for hiding this comment

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

A couple small adjustments here:

  • Handle 0 results appropriately
  • clean up ElationCollection / Find duplication

extensions/elation/actions/findPhysician.ts Outdated Show resolved Hide resolved
extensions/elation/client.ts Outdated Show resolved Hide resolved
extensions/elation/types/generic.ts Show resolved Hide resolved
@michal-grzelak michal-grzelak force-pushed the elation-find-physician branch 2 times, most recently from df01ad6 to 8fbcb0a Compare May 4, 2023 07:59
bejoinka
bejoinka approved these changes May 9, 2023
View reviewed changes
Copy link
Contributor

@bejoinka bejoinka left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good. I'm going to approve without validation of the fields because undefined values are removed from params in axios. (e.g. params: {foo: undefined, bar: 'baz'} evaluates to ?bar=baz)

@bejoinka bejoinka mentioned this pull request May 10, 2023
Merged
@bejoinka
Copy link
Contributor

oh, @michal-grzelak , totally missed it, but let's conform to what you did with #114 in this as well, and then we can merge 👍

@michal-grzelak
Copy link
Contributor Author

@bejoinka

oh, @michal-grzelak , totally missed it, but let's conform to what you did with #114 in this as well, and then we can merge 👍

Hi, in #115 there is everything finished with those changes as well. You can see that elation-consistency branch has a base of elation-find-physician branch.

bejoinka
bejoinka approved these changes May 22, 2023
View reviewed changes
Copy link
Contributor

@bejoinka bejoinka left a comment

Choose a reason for hiding this comment

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

This is in good shape to merge once we get approval from the business side.

@bejoinka
Copy link
Contributor

Okay, we're good to merge as soon as we rebase and use the new extensions core package.

@michal-grzelak -- do you want to pick this one back up once you're free?

@michal-grzelak
Copy link
Contributor Author

@bejoinka Sure, I can can take it when I finish the current task

@michal-grzelak
Copy link
Contributor Author

@bejoinka This PR is ready to be merged from my side. Old code removed and using @awell-health/extensions-core

@michal-grzelak @bejoinka
feat(): add physician typing
e1fed3f
@michal-grzelak @bejoinka
feat(): add client request to list physicians
b92e35b
@michal-grzelak @bejoinka
refactor collection types
190bcd8
@michal-grzelak @bejoinka
feat(): add findPhysician action
646b2bd
@michal-grzelak @bejoinka
fix(): use correct url
2aac6df
@michal-grzelak @bejoinka
feat(): add findPhysician test
383374f
@michal-grzelak @bejoinka
feat(): add query params to request
274c04b
@michal-grzelak @bejoinka
feat(): cr changes
4bcfd58
@michal-grzelak @bejoinka
Elation chart notes (#112)
ddf4a5f 
* feat(): add chart notes validation

* feat(): add non-visit note endpoints

* feat(): add createNonVisitNote action

* feat(): add tests for create note

* feat(); refactor validation

* feat(): add update note action

* feat(): add get note action

* feat(): add delete note action

* feat(): use patch instead of put

* feat(): update tags validation

* add update note test

* feat(): add delete note test

* feat(): add get note tests

* feat(): fix errors and update tests

* feat(): refactor update logic

* feat(): add optional category field

* feat(): cr changes
@michal-grzelak @bejoinka
feat(): fix chart notes consistency
f5b21c8
@michal-grzelak @bejoinka
feat(): fix create appointment consistency
b592fcc
@bejoinka bejoinka merged commit b9b5bfe into main Jun 30, 2023
1 check passed
@bejoinka bejoinka deleted the elation-find-physician branch June 30, 2023 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bejoinka bejoinka bejoinka approved these changes

@ebomcke-awell ebomcke-awell Awaiting requested review from ebomcke-awell

Assignees

@michal-grzelak michal-grzelak

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Elation: Find Physician
2 participants
@michal-grzelak @bejoinka