Implementation of Consent Manager 2.0 + fixes after reviews #85

simhue · 2025-09-25T11:12:09Z

No description provided.

…e.Instant serialization

Angular Workspace migration. Update an Angular CLI workspace to version 9.

- optimized build inludes/excludes - refactoring - add new classes for master data and encounters - apply new logo and icons

- add comments - refactoring - implement batch registration

fixes after first review #3 (comment)

…as not working

- add case for http 304 code in error interceptor - refactor request polling

Signed-off-by: Simon H. <huening.simon@gmail.com>

akomii

Review WIP. Only Java code done so far. Project not buildable via mvn:

[ERROR] ▲ [WARNING] The glob pattern import("./**/*.entry.js*") did not match any files [empty-glob]
[ERROR] 
[ERROR]     node_modules/@stencil/core/internal/client/index.js:169:2:
[ERROR]       169 │   `./${bundleId}.entry.js${BUILD5.hotModuleReplacement && hmrVers...
[ERROR]           ╵   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[ERROR] 
[ERROR] 
[ERROR] ✘ [ERROR] Could not resolve "semantic/dist/semantic.min.css"
[ERROR] 
[ERROR]     angular:styles/global:styles:2:8:
[ERROR]       2 │ @import 'semantic/dist/semantic.min.css';
[ERROR]         ╵         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[ERROR] 
[ERROR]   You can mark the path "semantic/dist/semantic.min.css" as external to exclude it from the bundle, which will remove this error and leave the unresolved path in the bundle.
[ERROR] 
[ERROR] 
[ERROR] ✘ [ERROR] Could not resolve "semantic/dist/semantic.js" [plugin angular-script-global]
[ERROR] 
[ERROR]

admin-gui/src/main/webapp/semantic/src/site/globals/site.variables

admin-gui/src/main/java/org/aktin/dwh/admin/optin/PatientEntriesRequestDTO.java

akomii · 2025-10-27T14:09:10Z

admin-gui/src/main/java/org/aktin/dwh/admin/optin/OptInEndpoint.java

+        PatientEntry pat = study.getPatientByID(ref, root, ext);
+        if (pat != null) {
+            log.log(Level.WARNING, "Cannot create entry, PatientEntry already exists.");
+            return Response.status(Status.CONFLICT).entity("Patient*in existiert bereits").build();


Do not sent human-readable error messages. These are endpoints. A machine interacts with them (frontend). The translation to human-readable should be done in Frontend to UI

admin-gui/src/main/java/org/aktin/dwh/admin/optin/OptInEndpoint.java

akomii · 2025-10-28T08:35:58Z

admin-gui/src/main/java/org/aktin/dwh/admin/optin/OptInEndpoint.java

+        Study study = this.getStudy(id);
+        return study.getPatientByID(ref, root, ext);


This logic should be in the service.

So you are calling in StudyManagerImpl an SQL Query to get all Studies
Then you loop to these studies to get a specific study
Then you loop through the found study to get a specific patient

Why not just add the method to StudyManager and implement a SQL Query that does this?

Because then I do not need to touch two additional packages (dwh-api and dwh-query) and the computational impact should be negligible - correct me if I'm wrong

simhue · 2025-10-30T09:22:19Z

Review WIP. Only Java code done so far. Project not buildable via mvn:

[ERROR] ▲ [WARNING] The glob pattern import("./**/*.entry.js*") did not match any files [empty-glob]
[ERROR] 
[ERROR]     node_modules/@stencil/core/internal/client/index.js:169:2:
[ERROR]       169 │   `./${bundleId}.entry.js${BUILD5.hotModuleReplacement && hmrVers...
[ERROR]           ╵   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[ERROR] 
[ERROR] 
[ERROR] ✘ [ERROR] Could not resolve "semantic/dist/semantic.min.css"
[ERROR] 
[ERROR]     angular:styles/global:styles:2:8:
[ERROR]       2 │ @import 'semantic/dist/semantic.min.css';
[ERROR]         ╵         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[ERROR] 
[ERROR]   You can mark the path "semantic/dist/semantic.min.css" as external to exclude it from the bundle, which will remove this error and leave the unresolved path in the bundle.
[ERROR] 
[ERROR] 
[ERROR] ✘ [ERROR] Could not resolve "semantic/dist/semantic.js" [plugin angular-script-global]
[ERROR] 
[ERROR]

During a merge a line got deleted by accident. I pushed a fix.

#85 (comment)

aktin/dwh-admin#85 (comment)

….java

…as error responses

rwm · 2025-11-12T10:49:37Z

admin-gui/pom.xml

                <version>3.1.0</version>
                <configuration>
-                    <warSourceExcludes>node_modules/**,**/*.ts,**/*.js.map,.idea/**,.vscode/**</warSourceExcludes>
+                    <warSourceExcludes>node_modules/**,.angular/**,*.json,*.md,.browserslistrc,.vscode/**,.idea/**,dist/**,public/**,src/**</warSourceExcludes>


das soll ja den sourcecode enthalten und nur die kompilierten Teile auslassen. Warum steht hier public/ und src/ als ausnahmen drin?

rwm · 2025-11-12T11:00:05Z

admin-gui/src/main/webapp/.gitignore

 debug.log
 *.js
 *.js.map
+semantic


Hier fehlen kommentare was semantic ist und warum das ausgenommen wird.

rwm · 2025-11-12T11:00:54Z

admin-gui/src/main/java/org/aktin/dwh/admin/optin/ValidationErrorType.java

+    ENCOUNTERS_NOT_FOUND, // No encounters in db found
+
+    // Processing States
+    VALID,


was für ein Fehler ist VALID???

akomii · 2025-11-12T10:46:51Z

admin-gui/src/main/java/org/aktin/dwh/admin/optin/OptInEndpoint.java

+    @Path("{studyId}/{sic}")
+    @GET
+    @Produces({MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON})
+    public PatientEntry getEntryBySic(@PathParam("studyId") String id, @PathParam("sic") String sic) throws IOException {


Endpoints should never throw an Exception... Where would it go?

see comment above

akomii · 2025-11-12T10:56:06Z

admin-gui/src/main/java/org/aktin/dwh/admin/optin/OptInEndpoint.java

+        PatientEntry pat = study.getPatientByID(ref, root, ext);
+        if (pat != null) {
+            throw OptInError.buildError(Status.CONFLICT, OptInErrorType.PATIENT_ALREADY_EXISTS, "Cannot create entry, PatientEntry already exists");
+        }
+
+        pat = study.getPatientBySIC(entry.sic);
+        if (pat != null) {
+            throw OptInError.buildError(Status.CONFLICT, OptInErrorType.SIC_ALREADY_EXISTS, "Cannot create entry, SIC {0} already exists", entry.sic);
+        }


Again, an Endpoint is machine readable. Instead of creating a new Pojo to differentiate existing patient and sics you should use:

throw new ClientErrorException(Status.CONFLICT);

Alternatively:

throw new ClientErrorException( Response.status(Status.CONFLICT) .type(MediaType.APPLICATION_JSON) .entity("{detail: OptInErrorType.SIC_ALREADY_EXISTS}") .build());

akomii · 2025-11-12T10:56:25Z

admin-gui/src/main/java/org/aktin/dwh/admin/optin/OptInEndpoint.java

+    @Path("{studyId}/{reference}/{root}{p:/?}{extension:.*}")
+    @GET
+    @Produces({MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON})
+    public Response getEntry(@PathParam("studyId") String id, @PathParam("reference") PatientReference ref, @PathParam("root") String root,


Endpoints should never throw an IOException... Where would it go?

Not my original code, we should adress that when the code base gets redesigned eventually

akomii · 2025-11-12T10:57:00Z

admin-gui/src/main/java/org/aktin/dwh/admin/optin/OptInEndpoint.java

+    public Response getEntry(@PathParam("studyId") String id, @PathParam("reference") PatientReference ref, @PathParam("root") String root,
+                                 @PathParam("extension") String ext) throws IOException {
+        Study study = this.getStudy(id);
+        return Response.ok(study.getPatientByID(ref, root, ext)).build();


If study not found, 200 is returned with an empty body

it should be a 404

getStudy throws a not found exception when empty, but not getPatientByID, I'm going to add it

akomii · 2025-11-12T11:01:25Z

admin-gui/src/main/java/org/aktin/dwh/admin/optin/OptInEndpoint.java

+     * @return list of studies
+     * @throws IOException
+     */
+    @Path("studies")


Better to just remove the Path annoation for consistency

GET optin/studies -> all studies
GET optin/2 -> all entries of study with id 2??

or change the "optin" to "studies" because optin/2 -> what is an optin with the id 2?

akomii · 2025-11-12T11:07:10Z

admin-gui/src/main/java/org/aktin/dwh/admin/optin/OptInEndpoint.java

+
+        PatientEntry oldEntry = study.getPatientByID(ref, root, ext);
+        if (oldEntry == null) {
+            throw OptInError.buildError(Status.NOT_FOUND, OptInErrorType.PATIENT_NOT_FOUND, "Patient not found");


Why do you need OptInError for this? Why not just:

throw new ClientErrorException(Status.NOT_FOUND);

akomii · 2025-11-12T11:19:48Z

admin-gui/src/main/java/org/aktin/dwh/admin/optin/OptInEndpoint.java

+            log.log(Level.WARNING, "Cannot update entry, entry does not exist");
+            return Response.status(Status.BAD_REQUEST).entity("Patient*in nicht gefunden").build();
+        }
+        PatientEntry newEntry = study.getPatientByID(ref, root, ext);


I still dont get it... It a PUT, its called updateEntry, but all it does it update the comment?

akomii · 2025-11-12T11:20:17Z

admin-gui/src/main/java/org/aktin/dwh/admin/optin/OptInEndpoint.java

+            throw OptInError.buildError(Status.NOT_FOUND, OptInErrorType.PATIENT_NOT_FOUND, "Patient not found");
+        }
+        pat.delete(security.getUserPrincipal().getName());
+        return Response.ok().build();


Commonly, NO_CONTENT / 204 is returned if an DELETE is successful

simhue added 30 commits August 28, 2024 10:46

Add com.fasterxml.jackson.datatype.jsr310.JavaTimeModule for java.tim…

044bd45

…e.Instant serialization

Angular CLI update for packages - @angular/core@9, @angular/cli@9

bbf630a

@angular/cli migration - workspace-version-9

3705f2e

Angular Workspace migration. Update an Angular CLI workspace to version 9.

remove obsolete files

a177260

upgrade to Angular 18

c947136

#3: first prototype of consent manager 2.0

f0122eb

Merge branch 'master' into Ugrade-to-Angular-18

d41f588

fix errors that occurred due to upgrade to angular 18

5482833

add lombok dependency

2bf2639

https://github.com/aktin/dwh-admin/issues/3:

104e9da

- optimized build inludes/excludes - refactoring - add new classes for master data and encounters - apply new logo and icons

remove obsolete file

dd0d4af

https://github.com/aktin/dwh-admin/issues/3:

2c7b93d

- add comments - refactoring - implement batch registration

https://github.com/aktin/dwh-admin/issues/3:

bd745f2

fixes after first review #3 (comment)

Merge branch 'develop' into consent-manager-20

4ee8274

feat: #67: add custom theme

892560e

feat: #67: add semantic sources to gitignore

659276c

feat: #67: make fomantic ui build process more ci/cd friendly

7477af6

feat: #67: force add site.variables since re-inclusion in gitignore w…

3f41174

…as not working

feat: #67: remove obsolete files

6f92322

feat: #67: change primary color

829bcb7

Merge branch 'theming' into cm2-with-theming

4ae3473

#49: - let error interceptor handle http errors

bba4379

- add case for http 304 code in error interceptor - refactor request polling

fix: fix request result download

3b4a753

fix: remove git merge tags

fef16f8

feat: #67: unify width of pages

38b53d3

33: reworked modal/dialog logic

7759c1f

change options

b98d44a

#67: move fomantic ui to dev dependencies

1abe136

pass error to console as object

73581dc

#67: remove obsolete attributes

98359e5

simhue and others added 2 commits October 23, 2025 11:26

Merge branch 'develop' into feat/consent-manager-20

b54a583

Signed-off-by: Simon H. <huening.simon@gmail.com>

updated aktin internal dependencies to current versions

c30cfce

akomii reviewed Oct 27, 2025

View reviewed changes

akomii reviewed Oct 28, 2025

View reviewed changes

reinsert accidentally deleted line

2e2ebd8

#85: use map instead of two lists

8aae471

#85 (comment)

simhue added a commit to aktin/dwh-query that referenced this pull request Nov 3, 2025

aktin/dwh-admin#85: use map instead of two lists

03849fe

aktin/dwh-admin#85 (comment)

simhue added a commit to aktin/dwh-api that referenced this pull request Nov 3, 2025

aktin/dwh-admin#85: use map instead of two lists

112aa43

aktin/dwh-admin#85 (comment)

simhue added 14 commits November 3, 2025 16:14

#85: remove obsolote method

9730fad

#85: fix method description

12e97fc

#85: rename PatientEntriesResponseDTO.java to PatientEntryResponseDTO…

0cc8ccc

….java

#85: add new class

fe86e2e

#85: fix sort order

6cab6c9

#85: rework http error handling, no more sending human readable text …

174295a

…as error responses

#85: delete old studyManager folder and sources

56d52ac

#85: add missing javadoc

a7f6b3f

#85: change all endpoint return types to Response

3d6a010

#85: remove unnecessary code

0a290e4

#85: remove unused imports

c5199f9

#85: remove provider

6d21b62

#85: use js veriable to store preferences instead of session storage

4b7349c

#85: change created response to ok

4739e5e

simhue added a commit to aktin/dwh-query that referenced this pull request Nov 10, 2025

aktin/dwh-admin#85: move sic generation from endpoint to service

4ae060e

#85: move sic generation from endpoint to service

97771b1

rwm reviewed Nov 12, 2025

View reviewed changes

akomii reviewed Nov 12, 2025

View reviewed changes

akomii linked an issue Nov 13, 2025 that may be closed by this pull request

Improve Code Quality of the Study Manager Endpoint #89

Open

7 tasks

		Study study = this.getStudy(id);
		return study.getPatientByID(ref, root, ext);

Implementation of Consent Manager 2.0 + fixes after reviews #85

Are you sure you want to change the base?

Implementation of Consent Manager 2.0 + fixes after reviews #85

Uh oh!

Conversation

simhue commented Sep 25, 2025

Uh oh!

akomii left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simhue commented Oct 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants