Concept Set Snapshots/Locking/Unlocking feature

[oleg-odysseus]

No description provided.

[chrisknoll]

Let's follow the existing database migration pattern for creating permissions. There are 2 forms:

1: You just have new permissions:, example here.

In this example, we create the tables, (if necessary) insert the new permissions into sec_permission, then assign those permissions to roles.

2: Copying permissions or updating existing permissions some way, example here.

In this example, we're creating a temp table temp_migration where we're going to migrate some existing permissions to new permissions, in the above case I think we're taking certain permissions from vocabulary/cdmresults endpionts and creating new permission for the access permission.

In another example (here), we're building a set of permissions based on another permission that is associated to source permissions. In this case, we're also using a temp table to set it up, and then insert into the permission table.

Both these examples have PRs associated to them (check the commit) which will explain the rationale for the change.

It's very important that we follow existing patterns about solving a particular problem in the codebase (in this case, creating permissions through a series of insert statements vs. what you implemented which appears to use cursors and loops). This is so that there is a common understanding and approach for solving a problem that is recognizable when we are looking at a particular piece of code. You don't need to check for the existance of a permission because there is a unique constraint set up on the sec_permission.value column, which will lead to a migration failure if the permission already exists.

[oleg-odysseus]

Hi @chrisknoll , thank you for the review and the examples with the standard approach to such migrations. I've changed it to use the temp table and tested by manually removing a conceptset:%s:snapshot:post permission and it`s role mapping for one of the concept sets and that SQL executed as a separate migration file inserted it back successfully. This means that for a fresh deployment my migration is expected to enrich each user's permissions for each concept set which has a conceptset:%s:update permission to have also a similar conceptset:%s:snapshot:post permission, that would allow snapshot actions for non-admin users who are owners of their existing concept sets (created before the lock feature was deployed - the snapshot button would be visible to them as well as the new snapshot action api operation).

[chrisknoll]

nd it`s role mapping for one of the concept sets and that SQL executed as a separate migration file inserted it back successfully. This means that for a fresh deployment my migration is expected to enrich each user's permissions for each concept set which has a conceptset:%s:update permission to have also a similar conceptset:%s:snapshot:post permission, that would allow snapshot actions for non-ad

Thanks @oleg-odysseus . It sounds like you were doing the Option 2 which is a copy permission use-case which makes sense. thanks for making the update.