Conversation
lruzicki
force-pushed
the
develop
branch
2 times, most recently
from
163076b to
e46fc08
Compare
lruzicki
force-pushed
the
main
branch
3 times, most recently
from
8471058 to
5607c00
Compare
dborowiecki
requested changes
Jul 10, 2023
dborowiecki
left a comment
dborowiecki
left a comment
There was a problem hiding this comment.
I left several comments. Views.py requires some rework, same for the services that are hardcoded for the openAPI compliance payloads. This should be refactored in order to handle more unstructured data.
govstack_test_harness_api/building_blocks/bb-digital-registries/insureequery.py
Outdated
Show resolved
Hide resolved
govstack_test_harness_api/building_blocks/bb-digital-registries/insureequery.py
Outdated
Show resolved
Hide resolved
| # from contribution_plan import schema as contribution_plan_schema | ||
|
|
||
|
|
||
| class QueryTest(TestCase): |
There was a problem hiding this comment.
Does this class test any logic? If it's just helper for GraphQL tests then you could make it a Mixin. Django TestCase is already a subclass of unittest TestCase, I don't think we should add another level of inheritance. Also it doesn't provide any logic that would be executed during setup. As it's only providing one function we could drop this class as a whole and just keep set_up_env function that would be used by tests independently.
govstack_test_harness_api/building_blocks/bb-digital-registries/insureequery.py
Outdated
Show resolved
Hide resolved
|
|
||
| response_data = {"status": 200} | ||
| response = HttpResponse(json.dumps(response_data)) | ||
| response['Content-Type'] = "application/json; charset=utf-8" |
There was a problem hiding this comment.
Shouldn't this header be defined somewhere on DRF level?
Comment on lines
+222
to
+229
| if 'errors' in result: | ||
| print(result['errors'][0]) | ||
| response_data = { | ||
| "answer": { | ||
| "status": 401, | ||
| "message": result['errors'][0] | ||
| } | ||
| } |
There was a problem hiding this comment.
In the separate client for GQL you can add separate method for handling errors.
Comment on lines
+368
to
+375
| context = login_with_env_variables(request) | ||
| client = get_client(Schema, Query, Mutation) | ||
|
|
||
| query = get_insurees_query(f'chfId: "{uuid}"', f"{field}") | ||
| result = client.execute(query, context=context) | ||
|
|
||
| insuree_data = result['data']['insurees']['edges'][0]['node'] | ||
|
|
There was a problem hiding this comment.
This part is very similar to one in previous view. Please make it more generic.
govstack_test_harness_api/building_blocks/bb-digital-registries/views.py
Outdated
Show resolved
Hide resolved
govstack_test_harness_api/building_blocks/bb-digital-registries/views.py
Outdated
Show resolved
Hide resolved
* TECH-717: draft endpoint that always return true added * TECH-717: module renamed to govstack_api * TECH-717: empty module created * TECH-779: changed function views to class views * TECH-779: added factory and apiviews class * TECH-779: added draft client api graphql * TECH-718: added middleware for info mediator client * TECH-718: added middleware for jwt token auth and login decorator * TECH-718: auth with token authorization * TECH-779: added graphql api client common for all building blocks * TECH-779: added insuree registry for api test scheme * TECH-779: get insuree endpoint completed * TECH-779: deleted insuree serializer to keep only those genric one * TECH-779: insuree registry methods for single registry * TECH-779: multiple serializer added * TECH-779: working version * TECH-779: default version * TECH-779: revert adding json file * TECH-718: added authentication (#11) Co-authored-by: lruzicki <lruzicki@gmail.com> * TECH-783: added class configuration (#10) * TECH-783: added class configuration * TECH-783: fixed missing argument for update function * TECH-783: refactorem insuree registry and exported most functions into base class * TECH-783: added views and controllers for multiple records * TECH-783: added registry creation to the admin panel * TECH-783: moved functions from insuree registry to base registry * TECH-783: moved functions from insuree registry to base registry * Feature/tech 794 (#12) * TECH-794: added tests for presence, update and reading single fields * TEHC-794: added test for single endpoints * TECH-794: added list multiple records endpoint * TECH-794: sorting for queries added * TECH-794: code refactor for sonar cloud * TECH-794: refactor for code smells in tests * test dummy validator * test dummy validator --------- Co-authored-by: lruzicki <lruzicki@gmail.com> --------- Co-authored-by: lruzicki <lruzicki@gmail.com> Co-authored-by: Damian Borowiecki <dborowiecki@soldevelo.com> * Refactor checkpoint * Refactor 2 * Refactor 3 * Refactor 4 * Refactor 5 * Refactor 6 * Refactor 6 * Refactor 7 * Refactor 8 * Added middleware for header fix * Removed old files --------- Co-authored-by: lruzicki <lruzicki@gmail.com> Co-authored-by: Patrick Delcroix <delcroip@gmail.com> Co-authored-by: Damian Borowiecki <dborowiecki@soldevelo.com> Co-authored-by: Damian Borowiecki <dborowiecki7@gmail.com>
…n the registry. It includes a predefined set of fields, default values, mutations, and queries, providing a starting point for the registry. Commit improves onboarding OpenIMIS into USCT use case https://github.com/GovStackWorkingGroup/sandbox-usecase-usct-backend/blob/main/docs/packages.md#packages
Snd 578 Add Initial Sample Record for 'InsureeRegistry' in the Registry
|
Kudos, SonarCloud Quality Gate passed!
|
Better error handling in middleware
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With this Pull Request, we introduce a new module for openIMIS. The newly created openIMIS backend module provides routing for the Building Block API in conjunction with the GovStack test suite. This addition enhances the functionality and interoperability of openIMIS, facilitating better testing mechanisms.