Activitylog fix #2168
Activitylog fix #2168
Conversation
Signed-off-by: muralibasani <[email protected]>
Signed-off-by: muralibasani <[email protected]>
Signed-off-by: muralibasani <[email protected]>
| @@ -144,25 +127,20 @@ public synchronized Map<String, String> insertIntoRequestConnector( | |||
| connectorRequest.setRequesttime((new Timestamp(System.currentTimeMillis()))); | |||
| kafkaConnectorRequestsRepo.save(connectorRequest); | |||
|
|
|||
| UserInfo userInfo = jdbcSelectHelper.selectUserInfo(connectorRequest.getRequestor()); | |||
| // ActivityLog activityLog = new ActivityLog(); | |||
There was a problem hiding this comment.
We should just delete this commented out code
| // operationalRequest.getTopicname() + "-" + operationalRequest.getConsumerGroup()); | ||
| // activityLog.setUser(operationalRequest.getRequestor()); | ||
| // activityLog.setEnv(operationalRequest.getEnvironment()); | ||
| // activityLog.setTenantId(operationalRequest.getTenantId()); |
There was a problem hiding this comment.
We should delete this commented out code
Signed-off-by: muralibasani <[email protected]>
| String requestor) { | ||
| ActivityLog activityLog = new ActivityLog(); | ||
| activityLog.setReq_no(getNextActivityLogRequestId(tenantId)); | ||
| activityLog.setActivityName(requestType + "Request"); |
There was a problem hiding this comment.
Does this "Request" need a space to separate the words e.g. " Request"
There was a problem hiding this comment.
Good one. Will do.
Signed-off-by: muralibasani <[email protected]>
coral/types/api.d.ts
Outdated
| @@ -556,7 +556,7 @@ export type components = { | |||
| errCode?: string; | |||
| message: string; | |||
| debugMessage?: string; | |||
| data?: Record<string, never>; | |||
| data?: unknown; | |||
There was a problem hiding this comment.
@mathieu-anderson is unknown acceptable here?
Signed-off-by: muralibasani <[email protected]>
| + "-" | ||
| + aclReq.getTopicname() | ||
| + "-" | ||
| + aclReq.getAcl_ssl() |
There was a problem hiding this comment.
I think but can't be 100% sure it is normally either acl_ssl or acl_ip depending on which type of acl it is do we just need to add a tertiary stmt here to decide onthe correct one.
| @@ -711,6 +712,14 @@ public ApiResponse approveConnectorRequests(String connectorId) | |||
| if (Objects.equals(updateConnectorReqStatus, ApiResultStatus.SUCCESS.value)) { | |||
| setConnectorHistory(connectorRequest, userDetails, tenantId); | |||
There was a problem hiding this comment.
I would consider putting the seConnectorHistory and InsertIntoActivityLog (for all of the variations kafka/connect/schema/acl) into their own methods as its a nice compartmentalized piece of code and with a good method name will be self explanatory about what is happening with a glance in this method.
| }); | ||
| } | ||
|
|
||
| public String getEnvName(String envId, String activityName, int tenantId) { | ||
| Optional<Env> envFound; | ||
|
|
||
| if ("SchemaRequest".equals(activityName)) { | ||
| if ("SchemaRequest".equalsIgnoreCase(activityName)) { |
There was a problem hiding this comment.
I know this is legacy but we probably should have these "SchemaRequest" and "ConnectorRequest" as class constants if we dont have an enum for them
There was a problem hiding this comment.
Nice suggestion. I flipped the condition abit
Signed-off-by: muralibasani <[email protected]>
Linked issue
Resolves: #2120
What kind of change does this PR introduce?
Only after approvals, activitylog entry is inserted
ActivityLogModel object with constraints is sent to FE now
[ X] Bug fix
New feature
[ X] Refactor
Docs update
CI update
What is the current behavior?
Describe the state of the application before this PR. Illustrations appreciated (videos, gifs, screenshots).
What is the new behavior?
Describe the state of the application after this PR. Illustrations appreciated (videos, gifs, screenshots).
Other information
Additional changes, explanations of the approach taken, unresolved issues, necessary follow ups, etc.
Requirements (all must be checked before review)
mainbranch have been pulledpnpm linthas been run successfully