[APPS-3152] fix for mnt-24137 #3102
Merged
Conversation
This file contains 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
SathishK-T
requested review from
jottley,
pzhyland,
kavitshah-gl and
tiagosalvado10
vedantmehra9
approved these changes
Dec 23, 2024
kavitshah-gl
approved these changes
Dec 23, 2024
jottley
approved these changes
Dec 23, 2024
| @@ -913,7 +913,7 @@ public int getAuditEntriesCountByAppAndProperties(AuditService.AuditApplication | |||
| final String applicationName = auditApplication.getKey().substring(1); | |||
There was a problem hiding this comment.
Maybe we can change this line to:
final String applicationName = auditApplication.getName();
There was a problem hiding this comment.
Changes are done as per the comment.
tiagosalvado10
requested changes
Dec 23, 2024
| @@ -913,7 +913,7 @@ public int getAuditEntriesCountByAppAndProperties(AuditService.AuditApplication | |||
| final String applicationName = auditApplication.getKey().substring(1); | |||
|
|
|||
| AuditQueryParameters parameters = new AuditQueryParameters(); | |||
| parameters.setApplicationName(applicationName); | |||
| parameters.setApplicationName(auditApplication.getName()); | |||
There was a problem hiding this comment.
Following the previous suggestion, we could supply the applicationName variable again here.
There was a problem hiding this comment.
Changes are done as per the comment.
|
I have added two suggestions in order to avoid having two different logic for the application name inside the same method. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
https://hyland.atlassian.net/browse/APPS-3129
This issue is happening when appNamePair shortvalue is generated with stringlowercase irrespective of caseSensitive flag, but as per the scenario the appName in audit application is differed by case (Upper vs Lower).
When its try to get appNamePair for appName its not matching value and its returning null from convertFromRestAuditQueryParameters(parameters) in getAuditEntriesCountByAppAndProperties method in AuditDAOImpl class.
So instead of getting auditEntries by the appKey we are now getting auditEntries by appName it will resolve the issue. Same approch already was there in getAuditEntry method in AuditImpl class.
Tested Scenarios:
We can’t use same AppName or AppKey for multiple applications which is already validated.
If we use same AppName or AppKey for multiple applications we are getting exception like org.alfresco.repo.audit.model.AuditModelException: 11200000 Audit application key 'testpermissionaudit' is used by: AuditApplication[ name=TESTPermissionaudit, id=3, disabledPathsId=4]