samvera-deprecated / curation_concerns

A Hydra-based Rails Engine that extends an application, adding the ability to Create, Read, Update and Destroy (CRUD) objects (based on Hydra::Works) and providing a generator for defining object types with custom workflows, views, access controls, etc.
Other
15 stars 27 forks source link

Add Sipity Workflow to CurationConcerns #1060

Closed jcoyne closed 7 years ago

jcoyne commented 7 years ago

@projecthydra/sufia-code-reviewers

jpstroop commented 7 years ago

I wonder if squashing this down into a few logical chunks (something less than 124) might make review a little easier. @escowles also suggested this might be a good topic for next week's tech call.

mjgiarlo commented 7 years ago

@tpendragon Would it be helpful if @jcoyne did what @jpstroop recommended above, or meh?

jcoyne commented 7 years ago

@tpendragon POKE have you had a chance to look at this?

tpendragon commented 7 years ago

@jcoyne Sorry, still looking at it. I don't mean to be holding everyone up, local priorities have been taking up my time.

tpendragon commented 7 years ago

In the workflow roles controller, the "roles" should definitely be a multi-select box.

tpendragon commented 7 years ago

Besides a couple tiny comments above, the code looks fine, and it works great. It looks like the workflow generator can be run multiple times without any issues, too.

However, a big thing this is missing for us is programmatic role assignment - some users of our application have roles as a side effect of some properties they have (in our case, all users can "flag"). Having it all in the database with no interface between the parts means I can't do that, at least as far as I can tell. The best I can do is a bunch of after_save hooks on users which auto-build roles. I'm concerned that the database structure is the interface. Thoughts?

jcoyne commented 7 years ago

@tpendragon I will work on supporting Groups as Agents this week. This should allow you to grant "flag" to any "registered" user, without callbacks.

tpendragon commented 7 years ago

@jcoyne I see you did that - with this, how does one assign a group to roles?

jcoyne commented 7 years ago

@tpendragon

group = CurationConcerns::Group.new('registered')
group_agent = group.to_sipity_agent
PermissionGenerator.call(roles: 'reviewing', workflow: sipity_workflow, agents: group_agent)