DASSL / ClassDB

An open-source system to let students experiment with relational data
https://dassl.github.io/ClassDB/
Other
8 stars 2 forks source link

The default object disposition assigns objects to a classdb role only to be dropped on uninstall (W) #191

Closed smurthys closed 6 years ago

smurthys commented 6 years ago

The default value assign_i for parameter objectsDisposition in function dropRole has the effect of accumulating a lot of objects under the ClassDB role classdb_instructor with the user having not being conscious of this outcome. However, later removing ClassDB from the DB just drops all objects owned by that ClassDB role.

I believe the following changes are worth considering:

These changes are somewhat easy to make and should be made in M2 to avoid having to patch data in a future release.

smurthys commented 6 years ago

Also, by extension, we should reject a ClassDB role's name as the value of parameter newObjectsOwnerName.

smurthys commented 6 years ago

Love to hear others' thoughts on this issue and the proposed resolution.

wildtayne commented 6 years ago

I think this resolution makes sense, assigning objects to classdb_instructor only really made sense with the old role system. Related to this, we should probably remove L52-L70 from removeAllFromDB.sql. I believe the purpose of this was to prevent the issue detailed here, however it doesn't appear to work for some reason - I tested running the uninstaller where some former student objects objects were owned by classdb_instructor and they were still dropped. With this new default behavior, and the new role base system, I think this code no longer serves much of a purpose.

afig commented 6 years ago

I agree that this solution is better. This should avoid the situation of not knowing the resulting ownership of objects that the user being dropped previously owned.

I was beginning to re-write the docs for Dropping Users, and thought about how it would be difficult to explain how the default object disposition worked without getting too in-depth about the "behind the scenes" implementation of ClassDB's group roles. (The page would have gotten more technical than it needed to be.)

smurthys commented 6 years ago

FYI, I expect to push a commit to fix this issue this Friday.