feat(remediation): user id is set for remediation#1134
Conversation
kanstantsinbuklis-sap
requested review from
MR2011,
copyonwrite,
dorneanu,
drochow and
michalkrzyz
as code owners
There was a problem hiding this comment.
Pull request overview
This PR updates remediation creation to persist a “unique user id” on newly created remediations, and adjusts E2E coverage to validate that the remediatedBy field is set in API responses.
Changes:
- Populate
RemediatedBy(unique user id) andRemediatedByIdduringCreateRemediationusing the current request context user. - Add
common.GetCurrentUniqueUserId(...)helper for extracting the unique user id from the auth context (or falling back to the system user). - Update E2E remediation creation tests to assert
remediatedByis not empty and stop sendingremediatedByin one of the create mutation inputs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| internal/app/remediation/remediation_handler.go | Sets RemediatedBy/RemediatedById during remediation creation based on the current user context. |
| internal/app/common/user_id.go | Adds helper to retrieve the current user’s unique user id (with system fallback). |
| internal/e2e/remediation_query_test.go | Updates E2E remediation create assertions around remediatedBy being set. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| Expect(*respData.Remediation.Vulnerability).To(Equal(remediation.Issue)) | ||
| Expect(*respData.Remediation.Image).To(Equal(remediation.Component)) | ||
| Expect(*respData.Remediation.RemediatedBy).To(Equal(remediation.RemediatedBy)) | ||
| Expect(*respData.Remediation.RemediatedBy).ToNot(BeEmpty()) |
| Expect(*respData.Remediation.Vulnerability).To(Equal(remediation.Issue)) | ||
| Expect(*respData.Remediation.Image).To(Equal(remediation.Component)) | ||
| Expect(*respData.Remediation.RemediatedBy).To(Equal(remediation.RemediatedBy)) | ||
| Expect(*respData.Remediation.RemediatedBy).ToNot(BeEmpty()) |
| remediation.UpdatedBy = remediation.CreatedBy | ||
| remediation.RemediatedById = remediation.CreatedBy | ||
|
|
||
| remediation.RemediatedBy, err = common.GetCurrentUniqueUserId(ctx, rh.database) |
internal/app/common/user_id.go
Outdated
| func GetCurrentUniqueUserId(ctx context.Context, db database.Database) (string, error) { | ||
| if authentication_context.IsAuthenticationRequired(ctx) { | ||
| uniqueUserId, err := authentication_context.UserNameFromContext(ctx) | ||
| if err != nil { | ||
| return "", fmt.Errorf("Could not get user name from context: %w", err) | ||
| } | ||
|
|
||
| return uniqueUserId, nil | ||
| } else { | ||
| return systemUserUniqueUserId, nil | ||
| } | ||
| } |
kanstantsinbuklis-sap
force-pushed
the
kanstantsinbuklis-sap/issue-1122/set-user-id-for-remediation
branch
from
f3c80b0 to
429215c
Compare
|
The In the mutation resolver, we need to check if a user with the unique user id exists, if not, we create a new user |
kanstantsinbuklis-sap
force-pushed
the
kanstantsinbuklis-sap/issue-1122/set-user-id-for-remediation
branch
from
429215c to
8a71838
Compare
| return nil, wrappedErr | ||
| } | ||
|
|
||
| remediation.CreatedBy = remediation.RemediatedById |
There was a problem hiding this comment.
The Remediation could be created by User A, but the RemediationBy is set to User B. The CreatedBy and UpdatedBy needs to be set to the authenticated user
kanstantsinbuklis-sap
force-pushed
the
kanstantsinbuklis-sap/issue-1122/set-user-id-for-remediation
branch
from
8a71838 to
1c1b1c9
Compare
MR2011
deleted the
kanstantsinbuklis-sap/issue-1122/set-user-id-for-remediation
branch
Co-authored-by: Michael Reimsbach <michael.reimsbach@sap.com>
Description
In this PR I've added saving
unique user idto remediationWhat type of PR is this? (check all applicable)
Related Tickets & Documents
feat(remediation): user id is set for remediation #1122
Added tests?
Little bit change test to check if the remediatedBy was set
Added to documentation?