Skip to content

Commit

Permalink
#721 | IsVoided for GroupPrivileges with IMPL_VERSION == 1 is meaning…
Browse files Browse the repository at this point in the history
…less, return false for them always
  • Loading branch information
himeshr committed Jul 18, 2024
1 parent b3abd8e commit 574fc4a
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -218,4 +218,13 @@ public int getImplVersion() {
public void setImplVersion(int implVersion) {
this.implVersion = implVersion;
}

/**
*
* @return For GroupPrivileges with IMPL_VERSION == 1 return FALSE, otherwise return actual db isVoided value .
*/
@Override
public boolean isVoided() {

This comment has been minimized.

Copy link
@mahalakshme

mahalakshme Jul 19, 2024

Contributor

@himeshr Here as well, I don't think we should change the behaviour. It should be whatever is in the db. The more are the ifs and buts, later when we need to support voided it will become confusing. Did something break without this?

This comment has been minimized.

Copy link
@himeshr

himeshr Jul 19, 2024

Author Contributor

This is as per our current design in GroupPrivileges, as we'll not be using isVoided flag anymore.
With IMPL_VERSION==1, we'll utmost have a single group_privilege whose allow field indicates whether its enabled or not.
So, to ensure we remember this fact, added this comment and override the method for clarity in future, as that what it means for all effective group_privileges which would have IMPL_VERSION==1

This comment has been minimized.

Copy link
@mahalakshme

mahalakshme Jul 19, 2024

Contributor

okay.

return getImplVersion() == GroupPrivilege.IMPL_VERSION ? false : super.isVoided();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public void savePrivileges(GroupPrivilegeContractWeb[] requests, Organisation or
List<ChecklistDetail> checklistDetails = checklistDetailRepository.findAll();
List<Group> groups = groupRepository.findAll();

Arrays.stream(requests).forEach(request -> {
Arrays.stream(requests).filter(grpPrivyConWebRequest -> !grpPrivyConWebRequest.isVoided()).forEach(request -> {

This comment has been minimized.

Copy link
@mahalakshme

mahalakshme Jul 19, 2024

Contributor

@himeshr I don't think we should change the behaviour of bundle upload unless really needed. In bundle upload our intention is to mirror the data in other org, It doesn't matter voided or not.

This comment has been minimized.

Copy link
@mahalakshme

mahalakshme Jul 19, 2024

Contributor

@himeshr Did something break without the above? U did it so that we dont sync allow true and voided true entries to client.

This comment has been minimized.

Copy link
@himeshr

himeshr Jul 19, 2024

Author Contributor

During testing, i made a manual modification in the bundle to have group_privilege with isVoided true flag, which i expect to be ignored and not used.

Its actually not expected to have such entries and isVoided flag is a dummy in general.

will revert this one check

GroupPrivilege groupPrivilege = groupPrivileges.stream().filter(gp ->
Objects.equals(request.getGroupUUID(), gp.getGroupUuid())
&& Objects.equals(request.getPrivilegeUUID(), gp.getPrivilegeUuid())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public static GroupPrivilegeContractWeb fromEntity(GroupPrivilege groupPrivilege
groupPrivilegeContractWeb.setEncounterTypeUUID(groupPrivilege.getEncounterTypeUuid());
groupPrivilegeContractWeb.setChecklistDetailUUID(groupPrivilege.getChecklistDetailUuid());
groupPrivilegeContractWeb.setAllow(groupPrivilege.isAllow());
groupPrivilegeContractWeb.setVoided(groupPrivilege.isVoided());
groupPrivilegeContractWeb.setNotEveryoneGroup(!groupPrivilege.getGroup().isEveryone());
return groupPrivilegeContractWeb;
}
Expand Down

0 comments on commit 574fc4a

Please sign in to comment.