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

Implemented initial design for the location based access control #2

Closed
wants to merge 1 commit into from

Conversation

suthagar23
Copy link
Member

Initial design for the location based access control project.

Description

This is an initial design for the location based access control which contains the basic implementation for module. It contains,

  1. Common methods for accessing person attributes
  2. Common methods for AOP Service
  3. Other some Common methods for the project.

Ticket

Ticekt : https://issues.openmrs.org/browse/LBAC-1

Others

  • I was able to compile the project without any failures.

@suthagar23
Copy link
Member Author

@dkayiwa Could you have a look at this PR?

*/
public interface AOPService extends Advisor {

boolean matches(Method method, Class targetClass);
Copy link
Member

Choose a reason for hiding this comment

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

I do not think we should create an interface when we do not yet have a use for it.

import org.openmrs.PersonAttribute;
import org.springframework.aop.framework.ReflectiveMethodInvocation;
import java.util.Set;

Copy link
Member

Choose a reason for hiding this comment

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

What is the use of this class?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a common class for the AOP Methods. We can use for users and patients while accessing the service methods.

@dkayiwa
Copy link
Member

dkayiwa commented May 24, 2018

Let us deal with these one step at a time. Can you start with a real use case from the end user's perspective? Then we implement what is needed to achieve it.

@dkayiwa
Copy link
Member

dkayiwa commented May 24, 2018

For instance, you can start with something as simple as registering a patient in a specific location. Do this in a way that we can even demo it for the end user. That way, we do not end up with classes and constants we may not use.

@suthagar23
Copy link
Member Author

Actually, I have tried only for Patients at the beginning, and then I made these common classes which can be used for the users as well. I have created PatientControlImpl class which used these common methods to save a location attribute while saving a patient.
Could you please have a look at here - https://github.com/openmrs/openmrs-module-locationbasedaccess/pull/4/files

Does this make sense for your question?

@dkayiwa
Copy link
Member

dkayiwa commented May 24, 2018

That is too much more than what am asking. Look at it this way. If a customer came and told you that all they need is to simply register patients with locations. That is the only code we need for now.

@suthagar23
Copy link
Member Author

Yah I got it well 😄 I will update the PR soon.

@suthagar23
Copy link
Member Author

@dkayiwa I moved to pull/4 with the new code base for only patients. Please have a look at there. Thanks.

@suthagar23
Copy link
Member Author

I'm closing this PR, and I will open it while starting work on this.

@suthagar23 suthagar23 closed this Jun 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants