Feature/gpcapim 251 #46
Conversation
DWolfsNHS
force-pushed
the
feature/GPCAPIM-251
branch
from
a73f501 to
89fa638
Compare
Vox-Ben
left a comment
Vox-Ben
left a comment
There was a problem hiding this comment.
Looks good, comments mostly on non-critical stuff. :-)
DWolfsNHS
force-pushed
the
feature/GPCAPIM-251
branch
from
2da189a to
2026888
Compare
|
ALB Target: |
davidhamill1-nhs
left a comment
davidhamill1-nhs
left a comment
There was a problem hiding this comment.
Happy with functionality. Some minor comments about style
Same comment as I left with @Vox-Ben , which needs a discussion amongst the team: to what extent do we document in code?
|
|
||
| headers = self._build_headers(trace_id) | ||
|
|
||
| endpoint_path = "/".join([ars_fhir_base, fhir_resource, ars_fhir_operation]) |
There was a problem hiding this comment.
Nit: I would say it is easier to parse ...
| endpoint_path = "/".join([ars_fhir_base, fhir_resource, ars_fhir_operation]) | |
| endpoint_path = f"{ars_fhir_base}/{fhir_resource}/{ars_fhir_operation}" |
| """ | ||
| provider_asid = "200000001154" | ||
| consumer_asid = "200000001152" | ||
| provider_endpoint = "https://invalid.com" |
There was a problem hiding this comment.
Nit: Given the assigned string, I would assume this to be an invalid url.
| provider_endpoint = "https://invalid.com" | |
| provider_endpoint = "https://test.com" |
|
ALB Target: |
3 similar comments
|
ALB Target: |
|
ALB Target: |
|
ALB Target: |
|
You've got one entire line not covered in coverage testing. Also you need to 
|
|
(I'm fine with 👍 this once the coverage and the sonar issue are sorted) |
|
ALB Target: |
Vox-Ben
left a comment
Vox-Ben
left a comment
There was a problem hiding this comment.
LGTM. One or two outstanding things to be discussed next week, but can be picked up as tech debt if we decide to do things differently.
DWolfsNHS
force-pushed
the
feature/GPCAPIM-251
branch
from
42041dd to
a673938
Compare
|
ALB Target: |
|
Completed walkthrough with @BJSS-russell-pollock ahead of merging |
|
3 participants
Description
This pull request introduces the GpProviderClient class, which provides a simple client for interacting with the GPProvider FHIR GP System. The client includes methods to fetch structured patient records from a GPProvider FHIR API endpoint. Additionally, unit tests and a stub for the GPProvider FHIR API have been implemented to ensure the correctness of the client.
Context
This change is required to enable integration with the GPProvider FHIR GP System, allowing the retrieval of structured patient records. It addresses the need for a robust and reusable client to interact with the GPProvider API, ensuring compliance with the GPConnect specifications.
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.