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.

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

Feature/cdt 186 add endpoint for execute stored aql queries #384

Merged

Conversation

markuswagnervitagroup
Copy link
Contributor

added handling of stored AQL queries to the SDK

Comment on lines 183 to 190
public QueryResponseData executeStoredQuery(
String qualifiedQueryName,
String version,
// Optional<UUID> ehrId, // TODO: ehr_id not available in openEHR 1.0.0 spec. Has to be included in
// queryParameter
Optional<Integer> offset,
Optional<Integer> fetch,
Optional<String> queryParameters) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to have a parameter object here.

Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion. use a dto with builder Patten. then in the its clear what is required and what is optional . and the returns can then return optinal for the optional values

Copy link
Contributor Author

Choose a reason for hiding this comment

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

parameter object added

String version,
// Optional<UUID> ehrId, // TODO: ehr_id not available in openEHR 1.0.0 spec. Has to be included in
// queryParameter
Optional<Integer> offset,
Copy link
Contributor

Choose a reason for hiding this comment

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

never use Optional as a parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -159,6 +159,24 @@ protected HttpResponse internalPost(
return response;
}

protected HttpResponse internalPut(
Copy link
Contributor

Choose a reason for hiding this comment

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

use this in httpPut line 184

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would recommend to use interalPut from httpPut because the method httpPut is using more header which are currently not needed for the stored query PUT

@sonarcloud
Copy link

sonarcloud bot commented Sep 5, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

87.6% 87.6% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@stefanspiska stefanspiska left a comment

Choose a reason for hiding this comment

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

@markuswagnervitagroup
Copy link
Contributor Author

Copy link
Contributor

@stefanspiska stefanspiska left a comment

Choose a reason for hiding this comment

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

update changelog

@sonarcloud
Copy link

sonarcloud bot commented Nov 25, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

87.6% 87.6% Coverage
0.0% 0.0% Duplication

@stefanspiska stefanspiska merged commit 0f625cb into develop Nov 25, 2022
@stefanspiska stefanspiska deleted the feature/CDT-186_add_endpoint_for_execute_stored_aql_queries branch November 25, 2022 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants