GSOC 2019 : Location Based Access Control Phase 2

Tags: #<Tag:0x00007f076c5109d0>

so that we don’t need to store the Global property.

If its an issue with performance I am fine with getByUUid and storing the GlobalProperty.

If it needed for some other implementations also, they can fetch it from GP. If we fixed it, then we only can make changes. So better to keep it

If someone wants to implement isn’t it the same they have to know the name of GLOBAL PROPERTY instead of PersonAttributeType name if I am correct?

Made Changes over this PR can you please review it.

yes, that will be also correct

  • Automating the LABC module installation,

Changes Made

  • Added PersonAttributeType to create a PersonAttribute which stores person location information.
  • Set the Globalproperty locationbasedaccess.locationAttributeUuid to Uuid of Added PersonAttributeType.
  • Above Changes are made in started method of LocationBasedAccessActivator class, to add those values while the installation of Module.

PR for this Issue at

can edit the Deployment page once PR gets Accepted. https://wiki.openmrs.org/display/docs/Location+Based+Access+Control+-+Deployment+Steps

Just added comments to the PR

  • Modify the current implementation to add one or more access locations to a user.

For this, I would make changes in the UI of creating User or Editing User Account as of that It has multiple checkboxes as like capabilities so that the user can have List of Location.

For this, the user should be having a property to store a list of UserLocations.

or

we can store every location with a property name as we only 8 different Locations.

or

do we create a table user_location to have the mapping.?

CC: @suthagar23, @dkayiwa

Let’s think, if a hospital has 50+ locations. What will happen? Is it alright to have checkboxes over there?

So, How can you fix tha - “OpenMRS can have only 8 locations as per we have now”. We have it just for a default sample :smile:.

do we create a table user_location to have the mapping.?

Is there any way of storing an array as the person attributes without lacking in the performance?

But doing so to get a list of users for a given location we need to check for every user whether if user belongs to this location or not by checking every location of him. makes the system slow if I am correct.

Why do we need such a reverse list here? Our use case is, get a list of accessible locations for a given user.

Did you find anyway to create a person attribute type with array format? Can you explore it more?

Maybe we may not but some other App might.

The user only has UserProperties but what PersonAttributes :thinking: are you pointing to.?

Anyway, we don’t need to consider such a requirement. We just need to focus on the requirements which are given for our module for now.

Without making any database changes, How can we achieve this?

For a person, we can have a list of PersonAttributes with the same PersonAttributeType.

But for a User what I got with is to have a user property with all locations uuids concatenated. like ’ location1+";"+location2+ " :" + location3 ’ as such.

Then how can we defer or modify those? So for the patients, there might not be any issues for adopting the multiple locations.

Is it the same way, Can we apply for user properties with same name?

Do you have any specific justificaitons for this rather than a simple comma based concatations?

Currently Person.class to add an attribute we have addAttribute(PersonAttribute newAttribute) which allows only to have one type of attribute.

To add attributes first we need to get all attributes list and then append the attributes to it and then use setAttributes(Set<PersonAttribute> attributes) method.

To modify or defer we need to first get all attributes then go through individual attribute check if it is a location attribute then either modify or defer it. But to get all location attributes we can use either of getAttributes(PersonAttributeType personAttributeType) or getAttributes(String attributeName) methods.

we cannot use, as user properties are stored in a map<String, String>.

sorry, It was a typo mistake I just mean the same to just concatenate them using some symbol so we can split use it.

Added few comments on the reviews.

Sounds good, So can you go ahead and test it - How far can we go with this approach? If it’s not worth then we need to think about another path

wouldn’t it be any issue with the performance.?