Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LBAC-14 Added restrictions to the PersonService methods to restrict them by the locations #20

Merged
merged 1 commit into from Aug 2, 2018

Conversation

@suthagar23
Copy link
Member

@suthagar23 suthagar23 commented Aug 2, 2018

Description

Added implementation to restrict the PersonService methods by the locations. These are the methods addressed in this PR,

  1. getPeople(query)
  2. getPerson(id)
  3. getPersonByUuid(uuid)

Ticket

Ticket : https://issues.openmrs.org/browse/LBAC-14

return (personAttribute != null && compare(personAttribute.getValue(), sessionLocationUuid));
}

public static Boolean compare(String value1, String value2) {

This comment has been minimized.

@dkayiwa

dkayiwa Aug 2, 2018
Member

Didn't we already have the compare method somewhere else?

This comment has been minimized.

@suthagar23

suthagar23 Aug 2, 2018
Author Member

Nope, I have just moved from the PatientSearchAdvicer class to LocationUtils calls (I need this for the PersonSearchAdvicer class also).

if (Daemon.isDaemonUser(Context.getAuthenticatedUser()) || Context.getAuthenticatedUser().isSuperUser()) {
return object;
}
Integer sessionLocationId = Context.getUserContext().getLocationId();

This comment has been minimized.

@dkayiwa

dkayiwa Aug 2, 2018
Member

This looks like a duplicate of what is in PatientSearchAdvice. Can't we refactor it for reuse?

This comment has been minimized.

@suthagar23

suthagar23 Aug 2, 2018
Author Member

Have you mentioned this line or the lines belong to this one?

This comment has been minimized.

@dkayiwa

dkayiwa Aug 2, 2018
Member

Everything in this method

This comment has been minimized.

@suthagar23

suthagar23 Aug 2, 2018
Author Member

I have analyzed this @dkayiwa

We can create a common method which can work for persons. So then we can use that to work for Patients. But we need to loop twice the patient list as following,

  1. To get the PersonList from the Patient List (common method only needs personList)
  2. The common method also loops the personList to check the locations.

So it may double the time of looping. Do you agree you go for this?

This comment has been minimized.

@dkayiwa

dkayiwa Aug 2, 2018
Member

Commit and i take a look.

This comment has been minimized.

@suthagar23

suthagar23 Aug 2, 2018
Author Member

I have added a different approach since I need to do the looping for three times earlier (Before calling commongPersonMtd, During the commonPersonMts, after returning from commonPersonMtd)

In the new approach, I have added a new class "AccessLocation" which can carry the needed data for the access. So I can able to reduce the amount of duplicate code in the classes.

This comment has been minimized.

@suthagar23

suthagar23 Aug 2, 2018
Author Member

Reverted to previous step @dkayiwa

@suthagar23 suthagar23 force-pushed the suthagar23:LBAC-14 branch from edd21bc to d57ef7b Aug 2, 2018
…hem by the locations

LBAC-14 Added restrictions to the PersonService methods to restrict them by the locations

LBAC-14 Added restrictions to the PersonService methods to restrict them by the locations
@suthagar23 suthagar23 force-pushed the suthagar23:LBAC-14 branch from d57ef7b to 44ebbe7 Aug 2, 2018
@dkayiwa dkayiwa merged commit ec5c401 into openmrs:master Aug 2, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@suthagar23
Copy link
Member Author

@suthagar23 suthagar23 commented Aug 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants