no privileges on ProgramWorkflowService.getWorkflow and getSate

Development
,
1

There is no

@Authorized( { PrivilegeConstants.GET_XXX })

above ProgramWorkflowService.getWorkflow and getSate

I cant see any workflow related privileges in PrivilegeConstants.

Should there be?

2

Since Workflow and State are subsidiary to Program, they should just use the GET_PROGRAMS and MANAGE_PROGRAMS privileges. (I.e. there’s no business value in giving someone access to only one of programs and workflows/states.)

So, yes, you should add the @Authorized annotations, using the privileges from Program.

I see that you added the get methods in this commit: https://github.com/openmrs/openmrs-core/commit/bd101b57c69def40ebf3b83310eaf55e74e997c7

Note that it was an intentional choice 9 years ago to not allow you to directly access Workflows and States in that service, but to make you access them via the Program, because we were experimenting (at the time) to see if we could move some responsibility from the services into the domain objects.

Since we didn’t continue down this path, I think it’s fair to just make this service consistent with the other ones, as you’re doing, but if you’re going to do this, I think you also need to add saveWorkflow and saveState methods. (Because otherwise what’s the point of being able to fetch a workflow/state by ID.)

3

thanks for the background info!

I am happy to add saveWorkflow and saveState, but

should this be discussed more?

I just found these bugs

https://issues.openmrs.org/browse/TRUNK-5157 https://issues.openmrs.org/browse/TRUNK-5154

and then that the service misses these methods. I can easily revert the changes and fix the bugs in another way.

4

@mseaton, since you were the driver of this particular refactoring, do you have any opinion about whether we should add saveWorkflow and saveState methods at this much later point?

5

@darius, I would encourage us to do whatever is best to make these services consistent with the rest of OpenMRS. The Program API is notoriously difficult to work with, and any steps to improving that would be welcome.

6

dear @mseaton,

can you please elaborate:

  • what makes it hard to work with?
  • what would you like it to do?

feel free to create a ticket describing for example what

  • saveWorkflow
  • saveState

should do.

I’ll help in implementing it :slight_smile: