Changed payloads for PartyAndUserLoginSecurityGroupDetails view perfo…#374
Changed payloads for PartyAndUserLoginSecurityGroupDetails view perfo…#374Banibrata-Manna wants to merge 1 commit intohotwax:mainfrom
Conversation
…rm find api calls Changelog: Changed
Banibrata-Manna
requested review from
ravilodhi and
ymaheshwari1
and removed request for
ymaheshwari1
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the query parameters for the PartyAndUserLoginSecurityGroupDetails entity by renaming roleTypeIdTo to roleTypeId and including it in the fieldList across several files. The review feedback identifies a significant risk in removing date-based filtering (filterByDate, fromDateName, and thruDateName), as this could lead to the retrieval of expired or historical records and result in incorrect data or inflated counts in the UI. Additionally, a suggestion was made to remove unnecessary quotes from the roleTypeId key to maintain consistency with the surrounding code.
| let userResp = {} as any, selectedUser = {} as any, params = { | ||
| inputFields: { | ||
| "roleTypeIdTo": "APPLICATION_USER", | ||
| "roleTypeId": "APPLICATION_USER", | ||
| partyId: payload.partyId, | ||
| }, | ||
| fromDateName: "relationshipFromDate", | ||
| thruDateName: "relationshipThruDate", | ||
| filterByDate: "Y", | ||
| viewSize: 1, | ||
| entityName: 'PartyAndUserLoginSecurityGroupDetails', | ||
| fieldList: ['createdByUserLogin', 'userLoginId', 'enabled', "hasLoggedOut", 'firstName', 'lastName', 'partyId', 'partyTypeId', 'groupName', 'externalId', 'statusId'], | ||
| fieldList: ['createdByUserLogin', 'userLoginId', 'enabled', "hasLoggedOut", 'firstName', 'lastName', 'partyId', 'partyTypeId', 'groupName', 'externalId', 'statusId', 'roleTypeId'], | ||
| } |
There was a problem hiding this comment.
Removing filterByDate: "Y" and the associated date field names (fromDateName, thruDateName) from the performFind payload for PartyAndUserLoginSecurityGroupDetails is a high-risk change. This entity typically involves time-sensitive associations (like UserLoginSecurityGroup). Without date filtering, the query may return expired or historical records, which can lead to duplicate entries or incorrect data being displayed in the UI. If this change is intended to improve performance, please ensure that the view definition itself is filtered to only return active records.
| const resp = await PermissionService.getSecurityGroupUsers({ | ||
| entityName: "PartyAndUserLoginSecurityGroupDetails", | ||
| noConditionFind: "Y", | ||
| fromDateName: "relationshipFromDate", | ||
| thruDateName: "relationshipThruDate", | ||
| filterByDate: "Y", | ||
| distinct: "Y", | ||
| viewSize: 1, | ||
| viewIndex: 0, | ||
| fieldList: ['partyId','securityGroupName'], | ||
| inputFields: { | ||
| securityGroupId: this.currentGroup.groupId, | ||
| roleTypeIdTo: "APPLICATION_USER" | ||
| roleTypeId: "APPLICATION_USER" | ||
| } | ||
| }) |
There was a problem hiding this comment.
Removing filterByDate: "Y" from the getSecurityGroupUsers call will cause the user count for a security group to include historical (expired) associations. This will result in an inaccurate, inflated count being displayed in the UI. If performance is a concern, consider if the backend view can be optimized while still maintaining correct filtering.
| const params = { | ||
| inputFields: { | ||
| roleTypeIdTo: 'APPLICATION_USER', | ||
| roleTypeId: 'APPLICATION_USER', | ||
| partyId: this.userProfile.partyId | ||
| }, | ||
| fromDateName: 'relationshipFromDate', | ||
| thruDateName: 'relationshipThruDate', | ||
| filterByDate: 'Y', | ||
| entityName: 'PartyAndUserLoginSecurityGroupDetails', | ||
| noConditionFind: 'Y', | ||
| distinct: 'Y', | ||
| fieldList: ['createdByUserLogin', 'createdDate', 'enabled', 'firstName', 'lastName', "groupName", 'partyId', 'securityGroupId', 'securityGroupName', 'statusId', 'userLoginId'], | ||
| fieldList: ['createdByUserLogin', 'createdDate', 'enabled', 'firstName', 'lastName', "groupName", 'partyId', 'securityGroupId', 'securityGroupName', 'statusId', 'userLoginId', 'roleTypeId'], | ||
| } |
There was a problem hiding this comment.
Removing filterByDate: 'Y' from the fetchLoggedInUserDetails payload may cause the application to retrieve expired security group associations for the user. This can result in the UI displaying outdated or incorrect security group information for the logged-in user.
| let userResp = {} as any, selectedUser = {} as any, params = { | ||
| inputFields: { | ||
| "roleTypeIdTo": "APPLICATION_USER", | ||
| "roleTypeId": "APPLICATION_USER", |
There was a problem hiding this comment.
The key "roleTypeId" is unnecessarily quoted, which is inconsistent with other keys in this object (like partyId on line 186) and other properties in the params object (like viewSize and entityName).
| "roleTypeId": "APPLICATION_USER", | |
| roleTypeId: "APPLICATION_USER", |
1 participant
Changed payloads for PartyAndUserLoginSecurityGroupDetails view perform find api calls
Changelog: Changed
Related Issues
#355