Skip to content

Commit

Permalink
add retire and make it transient
Browse files Browse the repository at this point in the history
  • Loading branch information
@mherman22
mherman22 committed Apr 17, 2023
1 parent e582a0f commit 2d80f75
Showing 1 changed file with 38 additions and 17 deletions.
55 changes: 38 additions & 17 deletions api/src/main/java/org/openmrs/Role.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.persistence.CascadeType;
import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.FetchType;
Expand Down Expand Up @@ -84,17 +83,45 @@ public class Role extends BaseOpenmrsObject {
)
@Cache(usage = CacheConcurrencyStrategy.READ_WRITE)
private Set<Role> childRoles;

@Transient
private Boolean retired = Boolean.FALSE;

@Transient
private String retireReason;

// Constructors

/** default constructor */
public Role() {
}

/** constructor with id */
public Role(String role) {
this.role = role;
}

/** constructor with all database required properties */
public Role(String role, String description) {
this.role = role;
setDescription(description);
}

public Boolean getRetired() {
return retired;
}

public void setRetired(Boolean retired) {
this.retired = retired;
}

public String getRetireReason() {
return retireReason;
}

public void setRetireReason(String retireReason) {
this.retireReason = retireReason;
}

/**
* @return the description
Expand All @@ -110,12 +137,6 @@ public void setDescription(String description) {
this.description = description;
}

/** constructor with all database required properties */
public Role(String role, String description) {
this.role = role;
setDescription(description);
}

/**
* @return Returns the privileges.
*/
Expand Down Expand Up @@ -154,15 +175,6 @@ public void addPrivilege(Privilege privilege) {
}
}

private boolean containsPrivilege(Collection<Privilege> privileges, String privilegeName) {
for (Privilege privilege : privileges) {
if (privilege.getPrivilege().equals(privilegeName)) {
return true;
}
}
return false;
}

/**
* Removes the given Privilege from the list of privileges
*
Expand Down Expand Up @@ -385,4 +397,13 @@ public Set<Role> recurseOverChildren(final Set<Role> total) {

return allRoles;
}

private boolean containsPrivilege(Collection<Privilege> privileges, String privilegeName) {
for (Privilege privilege : privileges) {
if (privilege.getPrivilege().equals(privilegeName)) {
return true;
}
}
return false;
}
}

12 comments on commit 2d80f75

@dkayiwa
Copy link
Member

@dkayiwa dkayiwa commented on 2d80f75 Apr 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to find out if roles have been retirable/voidable before, feel free to check the database table to see if it has these fields.

@mherman22
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i actually did that and thats why i have made sure theyre not persisted. I guess my confusion is on the rest side especially how to handle https://rest.openmrs.org/#delete-a-role. In the previous implementation, i believe delete was done by https://github.com/openmrs/openmrs-module-webservices.rest/blob/893152380641452f75696a7fc35609fcbb7613a0/omod-common/src/main/java/org/openmrs/module/webservices/rest/web/resource/impl/MetadataDelegatingCrudResource.java#L128-L137. So my thinking here is to be able to have the same implementation for the role resource.

@dkayiwa
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was that delete done for Role?

@mherman22
Copy link
Member Author

@mherman22 mherman22 commented on 2d80f75 Apr 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in the roleresource1_8 class, we instead have purge method

@dkayiwa
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purge method is simply doing this: Context.getUserService().purgeRole(role)

@mherman22
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purge method is simply doing this: Context.getUserService().purgeRole(role)

Yes, since i am extending the DelegatingCrudResource class implementing delete is inevitable and i was informed by these two test cases

@dkayiwa
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those two tests are not correct, because the Role property does not have any retire fields in the database. So it does not save them.
If you put these two lines after handle(req), the test would fail.

Context.flushSession(); Context.clearSession();

@mherman22
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that makes sense!
Lemi correct all this in my next commit

@mherman22
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dkayiwa Should the delete method return the resourcenotsupported exception?

@dkayiwa
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you point me to the exact delete method that you are talkin about?

@mherman22
Copy link
Member Author

@mherman22 mherman22 commented on 2d80f75 Apr 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you point me to the exact delete method that you are talkin about?

https://github.com/openmrs/openmrs-module-webservices.rest/blob/9c492c03034bbd563a21d7cc27b121e9e5339430/omod-1.8/src/main/java/org/openmrs/module/webservices/rest/web/v1_0/resource/openmrs1_8/RoleResource1_8.java#L303

@dkayiwa
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For that one we can throw an UnsupportedOperationException

Please sign in to comment.