-
Notifications
You must be signed in to change notification settings - Fork 1
Add create-by audit for scenario creation (again) #13
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
base: main
Are you sure you want to change the base?
Conversation
return await controller.create_scenario( | ||
scenario, | ||
request.state.user.userId if request.state.user else None, | ||
request.state.realm if request.state.realm else None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the LHA/Org IDs need to be mapped to the county codes.
IIRC, Mariama/Jonas mentioned that this mapping already exists. Maybe @JonasGilg can point us to it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mapping LHA IDs to county codes (or list of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Jackie,
you need to add the UserId
and OrgId
to the App's models (at least in /ESID-Backend/api/src/api/app/models/scenario.py
) otherwise it will not get returned in the API request but only stored in the DB.
You can still pass it along until the db task and put it in there, since the request only returns the id anyways, or write both into the scenario object as soon as you have the info, both is fine.
Finally, you then also need to fix the scenario_get_by_id
task to write the additional ids from the db into the app's Scenario
DTO so they get returned properly.
userId: Optional[str] = Field(default=None, nullable=True) # Created by user | ||
orgId: Optional[str] = Field(default=None, nullable=True) # Created by user's LHA/Organization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is the DB model this should probably be a UUID
.
userId: Optional[str] = Field(default=None, nullable=True) # Created by user | |
orgId: Optional[str] = Field(default=None, nullable=True) # Created by user's LHA/Organization | |
CreatorUserId: Optional[uuid.UUID] = Field(default=None, nullable=True) | |
CreatorOrgId: Optional[uuid.UUID] = Field(default=None, nullable=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User IDs from Keycloak are UUIDs but Org IDs are just strings.
|
Record user IDs and realm/LHA/Org IDs when creating new scenario. User IDs and Org IDs are extracted from the authentication middleware. I only changed the db model for Scenario but not any DTOs in between (I just changed the signature of those functions in controller and tasks). So far the User IDs are UUIDs generated by Keycloak, not username but we can change that upon request and the Org IDs are just the realm IDs on Keycloak as well. I think we might need to map those onto county code IDs?
Sorry for the weird commit and revert I made on main branch. I accidentally committed there.