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

Migrate some calls to FHIR and GraphQL #86

Merged
merged 24 commits into from Oct 18, 2023
Merged

Migrate some calls to FHIR and GraphQL #86

merged 24 commits into from Oct 18, 2023

Conversation

bmonjoie
Copy link
Contributor

The new implementation relies on OkHttp and Apollo.
OkHttp was added because the current implementation relies on Apache HTTP client, which is not supported anymore since Android 6 (API 23). A version supports Android 23+, but Apache does not officially maintain it. Also, OkHttp is the current standard and comes with lots of benefits, like, among other things, the logger, automatic support for GZip, and a strong security focus.
Apollo is in its version 2 as version 3+ requires Kotlin Coroutines. Also, the plan is that some new endpoints or extensions could be added to the FHIR API to remove the dependency on GraphQL.

Everything is hidden behind use cases to impact the current code base as little as possible.

Also, the code was migrated to AndroidX because of Apollo's dependencies.
The versions of the AndroidX have been kept back to avoid conflicts and upgrades.

dragos-dobre and others added 17 commits November 28, 2019 10:52
@dragos-dobre
Finish release/v2.0.0
18e2969 
Finish release/v2.0.0
@dragos-dobre
Finish release/v2.0.1
63d4a2f 
Finish release/v2.0.1
@dragos-dobre
Finish Release/v2.0.1
e772b24 
Finish Release/v2.0.1
@edarchis
Github Action: manual build
05fd9da
@dragos-dobre
Merge pull request #50 from openimis/manual-build-action
d1bedac 
Github Action: manual build
@dragos-dobre
Finish release/v2.1.0
5be00ee 
Release v2.1.0
@dragos-dobre
Update activity_about.xml
a993a46
@malinowskikam
Updated java version in github actions
ff292a4
@dragos-dobre
Finish feature/github_action_update
2ac6a91 
Github actions Java update
@malinowskikam
github actions: Changes cache from maven to gradle
38221be
@dragos-dobre
Finish feature/github_actions_fix
71ffbe1 
Github actions fix
@dragos-dobre
Merge pull request #80 from openimis/develop
0352400 
Develop
@dragos-dobre
Finish release/2.2.0
72f5f35 
Release/2.2.0
@dragos-dobre
Finish feature/OTC-745
a7bdaf9 
OTC-745 Implemented claim 2-way sync
@malinowskikam
Revert "OTC-745 Implemented claim 2-way sync"
d60c254
@delcroip
Merge pull request #85 from openimis/revert-83-feature/OTC-745
98ed387 
Revert "OTC-745 Implemented claim 2-way sync"
@bmonjoie
Migrate some calls to FHIR and GraphQL (WIP)
1726fa8 
The new implementation relies on OkHttp and Apollo.
OkHttp was added because the current implementation relies on Apache HTTP
client, which is not supported anymore since Android 6 (API 23).
A version supports Android 23+, but Apache does not officially maintain it.
Also, OkHttp is the current standard and comes with lots of benefits like,
among other things, the logger, automatic support for GZip, and a strong
security focus.
Apollo is in its version 2 as version 3+ requires Kotlin Coroutines.
Also, the plan is that some new endpoints or extensions could be added to the
FHIR API to remove the dependency on GraphQL.

Everything is hidden behind use cases to impact as less as possible the
current code base.

Also, migrate to AndroidX because of Apollo's dependencies. The versions
of the AndroidX have been kept back to avoid conflicts and upgrades.
@bmonjoie bmonjoie self-assigned this Jun 23, 2023
@bmonjoie
Copy link
Contributor Author

This PR depends on:

@bmonjoie
Moving the claim creation to FHIR (WIP)
53be603 
This depends on a modification of the backend which would allow
to pass a reference with an OpenIMIS Code rather than a UUID.

There is still an open question about the "REJECTED" status to be solved.

OMT-339
@dragos-dobre
Copy link
Member

The second link you mention is a branch and not a PR.

@toch
Copy link

toch commented Jun 28, 2023

Benjamin told me that the main changes are in directories domain, network, and usecase. He advices to start with network. As I'm working on the backend to support some of the needs, I'll go through this PR too.

@bmonjoie
Moving GetControls to GraphQL
53a20b1 
Deleting `ToRestApi` and removing Apache HTTP dependencies.
Adding a way to change the "Login/Logout" button automatically on login.

This depends on a new module migrating the controls to GraphQL that has not
yet been merged.

OMT-339
@bmonjoie
Improve "Add claim" UI
d3412c2
@bmonjoie bmonjoie force-pushed the OMT-339_migrate_to_FHIR_API branch from 737dd73 to d3412c2 Compare June 29, 2023 12:01
toch
toch reviewed Jun 30, 2023
View reviewed changes
Copy link

@toch toch left a comment

Choose a reason for hiding this comment

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

@bmonjoie Here a first review mainly on network, so you don't have to wait I've finished to read everything. I'm reviewing the rest.

I hope to have expressed clearly, smoothly, and politely my questions, suggestions, and concerns. I consider that I don't have all the cards in hand as you do, I hope this will help.

claimManagement/src/main/java/org/openimis/imisclaims/network/dto/ActivityDefinitionDto.java Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/network/dto/DiagnosisDto.java Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/network/dto/IdentifierDto.java Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/network/dto/LoginDto.java Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/network/dto/ActivityDefinitionDto.java Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/network/response/PaginatedResponse.java Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/network/request/BaseFHIRGetRequest.java Outdated Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/network/request/BaseFHIRRequest.java Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/network/request/BaseGraphQLRequest.java Outdated Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/network/request/PostNewClaimRequest.java Outdated Show resolved Hide resolved
toch
toch reviewed Jun 30, 2023
View reviewed changes
Copy link

@toch toch left a comment

Choose a reason for hiding this comment

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

second review, focused on domain and usecase. there is still java.org.openimis.imisclaims to read.

claimManagement/src/main/graphql/org.openimis.imisclaim/schema.graphqls Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/ImisActivity.java Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/SQLHandler.java Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/SQLHandler.java Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/Token.java Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/usecase/FetchInsureeInquire.java Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/usecase/FetchClaims.java Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/usecase/FetchClaims.java Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/domain/entity/PendingClaim.java Outdated Show resolved Hide resolved
claimManagement/src/main/java/org/openimis/imisclaims/domain/entity/PendingClaim.java Show resolved Hide resolved
@toch
Copy link

toch commented Jul 4, 2023

I've reviewed your answers. I can't resolve the discussions 🤷 so I've 👍 or answered them to do so.

@bmonjoie
Fix issues after first review
27aaf5a
@bmonjoie
Detect "REJECTED" status
8230867 
Based on the method `build_fhir_total_list` in the Python code,
if there is no total, the claim is rejected.
@bmonjoie
Fix error message when Insuree is not found
a196bad
@tdethier tdethier marked this pull request as ready for review September 25, 2023 12:36
@dragos-dobre
Copy link
Member

@bmonjoie We don't integrate code directly to main branch. Our development workflow follows GitFlow. Please create a PR for develop branch. I'm closing this PR.

@bmonjoie bmonjoie reopened this Oct 12, 2023
@bmonjoie bmonjoie changed the base branch from main to develop October 12, 2023 10:22
@bmonjoie
Copy link
Contributor Author

@dragos-dobre I have reopened the PR and changed the destination branch not to lose the history

@dragos-dobre dragos-dobre merged commit ae0015e into develop Oct 18, 2023
2 checks passed
@dragos-dobre dragos-dobre deleted the OMT-339_migrate_to_FHIR_API branch October 18, 2023 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@toch toch toch left review comments

@dragos-dobre dragos-dobre dragos-dobre approved these changes

@edarchis edarchis Awaiting requested review from edarchis

@tdethier tdethier Awaiting requested review from tdethier

Assignees

@bmonjoie bmonjoie

Labels
None yet
Projects
None yet
Milestone
No milestone
6 participants
@bmonjoie @dragos-dobre @toch @edarchis @malinowskikam @delcroip