304 add api for automated scheduled encounters - #314
Open
aluapaula wants to merge 18 commits into
Open
Conversation
…ller, to provide better maintainability and readability
…ller, to provide better maintainability and readability.
…ller, to provide better maintainability and readability.
…ller, to provide better maintainability and readability.
…ces after saveEncounter refactoring
Test Results🧪 Unit Test Results
🌐 Selenium Test ResultsRunning Test:
|
Test Results🧪 Unit Test Results
🌐 Selenium Test ResultsRunning Test:
|
Test Results🧪 Unit Test Results
🌐 Selenium Test ResultsRunning Test:
|
Test Results🧪 Unit Test Results
🌐 Selenium Test ResultsRunning Test:
|
- add @PostMapping endpoint to EncounterController - ad mapFromApiRequest() in EncounterScheduledDTOMapper - add new DTO EncounterScheduledApiRequestDTO
Test Results🧪 Unit Test Results
🌐 Selenium Test ResultsRunning Test:
|
- add @PostMapping endpoint to EncounterController - ad mapFromApiRequest() in EncounterScheduledDTOMapper - add new DTO EncounterScheduledApiRequestDTO
- add @PostMapping endpoint to EncounterController - add EncounterControllerTest
Test Results🧪 Unit Test Results
🌐 Selenium Test ResultsRunning Test:
|
Test Results🧪 Unit Test Results
🌐 Selenium Test ResultsRunning Test:
|
Test Results🧪 Unit Test Results
🌐 Selenium Test Results |
… EncounterScheduledApiRequestDTO
Test Results🧪 Unit Test Results
🌐 Selenium Test ResultsRunning Test:
|
ywarnecke
requested changes
Jul 14, 2026
ywarnecke
left a comment
ywarnecke
left a comment
Member
There was a problem hiding this comment.
The Changes are already working well. I have commented some needed adjustments / fixes.
Comment on lines
+7
to
+11
| -- Configuration may change with changing groups added. Please make sure this fits to your server config. | ||
| -- Adjust configuration_group_id to fit to your FHIR config group. Copy the values for all available FHIR configs on the server | ||
| -- You can find the ids with this query: | ||
| -- SELECT id FROM moPat.configuration_group WHERE configuration_group.label_message_code LIKE '%FHIR'; | ||
| -- On a server with no additional config groups, the id should be 11 |
Member
There was a problem hiding this comment.
Please adjust this. It is not specific to FHIR anymore
| return "redirect:/encounter/list"; | ||
| String apiKey = configurationDao.getApiKey(); | ||
|
|
||
| if (configurationDao.isApiKeyAccessEnabled() && apiKey.equals(key)) { |
Member
There was a problem hiding this comment.
Please adjust this function: Before anything happens on the server, it should be checked if the function is enabled at all. Move isApiKeyAccessEnabled() check to the beginning of the function
| break; | ||
| case API_KEY: | ||
| String apiKeyValue = configurationDTO.getValue(); | ||
| String apiKeyPattern = "^[A-Za-z0-9_\\-.]{16,128}$"; |
Member
There was a problem hiding this comment.
16 chars is probably too short. Please adjust to at least 32
| -- SELECT id FROM moPat.configuration_group WHERE configuration_group.label_message_code LIKE '%FHIR'; | ||
| -- On a server with no additional config groups, the id should be 11 | ||
| INSERT INTO moPat.configuration (`id`, `type`, `configuration_group_id`, `parent`, `position`, `attribute`, `configuration_type`, `description_message_code`, `class`, `label_message_code`, `test_method`, `update_method`, `uuid`, `value`, `pattern`) VALUES | ||
| (@largest_id + 1, 'GENERAL', 1, NULL, 17, 'enableApiTokenAccess', 'BOOLEAN', 'configuration.label.enableApiTokenAccess', 'GLOBAL', 'configuration.label.enableApiTokenAccess', NULL, NULL, '25fd8f5e-2e90-4cd1-8a30-d84c88889c5c', false, NULL), |
Member
There was a problem hiding this comment.
Please adjust the description message. It should not be the same as the label. Also for this feature, the description should be a disclaimer, how this feature opens up the application and allows everybody with the API key to create new encounters.
| configuration.file.path=Path to the uploaded file | ||
| configuration.file.uploaded=File uploaded | ||
| configuration.label.apiKey=API-Key for automated scheduled encounters | ||
| configuration.label.enableApiTokenAccess=Enablement of API-Key for automated scheduled encounters |
Member
There was a problem hiding this comment.
The encounters are not necessarily automated. Please adjust this wording
| -- On a server with no additional config groups, the id should be 11 | ||
| INSERT INTO moPat.configuration (`id`, `type`, `configuration_group_id`, `parent`, `position`, `attribute`, `configuration_type`, `description_message_code`, `class`, `label_message_code`, `test_method`, `update_method`, `uuid`, `value`, `pattern`) VALUES | ||
| (@largest_id + 1, 'GENERAL', 1, NULL, 17, 'enableApiTokenAccess', 'BOOLEAN', 'configuration.label.enableApiTokenAccess', 'GLOBAL', 'configuration.label.enableApiTokenAccess', NULL, NULL, '25fd8f5e-2e90-4cd1-8a30-d84c88889c5c', false, NULL), | ||
| (@largest_id + 2, 'GENERAL', 1, 17, 18, 'apiKey', 'STRING', 'configuration.label.apiKey', 'GLOBAL', 'configuration.label.apiKey', NULL, NULL, '9b48f864-8cb7-4704-82e2-12045755e2aa', '', NULL); |
Member
There was a problem hiding this comment.
The parent has to be adjusted. It needs to be @largest_id +1
Member
There was a problem hiding this comment.
Also adjust these changes in the other db scripts
| new Timestamp(getTodayAtMidnight().getTime()) | ||
| ); | ||
|
|
||
| return encounter; |
Member
There was a problem hiding this comment.
You missed a crucial step here; The bundle needs to be merged, so that the creation of the encounter can be cascaded. Right now, no new encounter is created, when sending out the email directly.
| } | ||
|
|
||
| @Override | ||
| public void validate(Object target, Errors errors) { |
Member
There was a problem hiding this comment.
It seems like locale needs to be present in the request, otherwise the application crashes without returning a bad request response. It should therefore be validated here. Alternatively check if present, otherwise set system locale (this should not be a side effect of validation though).
| * enable automatic scheduled encounters through an API | ||
| * @param request | ||
| * */ | ||
| @PostMapping(value = "/encounter/schedule/api") |
Member
There was a problem hiding this comment.
The api works well. However, it is quite easy to flood the server with data, if something goes wrong. It would be a good idea to add a rate limiter, e.g. with Bucket4j here. Something like this: https://bucket4j.com/8.18.0/toc.html#limiting-the-rate-of-access-to-rest-api
Collaborator
Author
There was a problem hiding this comment.
The rate limiting filter has been implemented. It applies to all requests targeting the API endpoint (authorized & unauthorized). The filter has two independent limits: a global limit shared across all requests, and an additional per-IP limit to prevent abuse from individual clients.
Test Results🧪 Unit Test Results
🌐 Selenium Test ResultsRunning Test:
|
Test Results🧪 Unit Test Results
🌐 Selenium Test ResultsRunning Test:
|
Test Results🧪 Unit Test Results
🌐 Selenium Test ResultsRunning Test:
|
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
2 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.
Refactoring of Method saveScheduledEncounter in class EncounterController, to provide better maintainability and readability.