Update role permissions after deactivation of role creates confusion

[nicolasfranck]

Please complete the following fields as applicable:

What version of the DMPRoadmap code are you running? (e.g. v2.2.0)

2.0.3

Expected behaviour:

If a user "removes" a plan, the plan isn't really removed.
Merely the associated "role" is internally flagged as "active=false".

The user no longer has access to the plan, but..

..other users that still have access still see this "role" in the share tab,
and they have no way of knowing that the role has been deactivated.
So they may alter the permissions, and unintentionally send
notifications to a user that lost interest?

Not sure about this

[briri]

Good catch @nicolasfranck. We should update that query to to only include active roles

[nicolasfranck]

and make sure that RolesController#create reuses the old deactivated Role record?
Otherwise an insert would fail

[briri]

@nicolasfranck would this be something you have the time to patch?

[nicolasfranck]

Some points:

• Apparently user uniqueness is not enforced on model level (Role), only in the RolesController. This controller skips Plan#add_user! altogether.
• Plan#add_user(user_id, access_type = :commenter) already reuses the old role, but does not activate it. Add this? Otherwise the reused role would not be visible. Although I do not see any places where this could happen, as this method is only used during creation of plans in PlansController#create and PlansController#duplicate

[nicolasfranck]

Similar question: it is not allowed to delete yourself in the share tab,
but you are allowed to deactivate your role using the link "remove",
which has basically the same effect. Intended?

[nicolasfranck]

What is actually the use case for using inactive roles in the first place? History?

[briri]

Yes, history and an edge-case scenario:

User A creates a plan and so becomes the 'owner' of the plan (creates the initial role record).
User A then invites user B to work on the plan as a 'co-owner' / 'administrator' (creates a role record)
User B invites user C to work on the plan as an 'editor' (creates a role record)
User A decides that the plan is 'complete' and makes its visibility public
After a year, user A decides to 'remove' the plan because they are tired of seeing it (deactivates the role)
User B also decides to do the same (deactivates the role)

Someone tries to download/view the public version of the plan.

When the plan is viewed/downloaded it needs to provide the Owner's info. If we delete the role records we cannot find that information. This also has implications in the API where we need to retain who the owner is.

[nicolasfranck]

Mm, but the public plan export uses the same code as the internal plan export, and that only takes active roles into account. See for example Plan#owner and Plan#owner_and_coowners. Am I missing something?

[briri]

correct, but there are places though where code like plan.owner.name and plan.owner.org are used and the nil causes errors. It also makes it impossible from a maDMP standpoint to determine the author of the DMP once everyone has been removed.

[nicolasfranck]

correct, but there are places though where code like plan.owner.name and plan.owner.org are used and the nil causes errors. It also makes it impossible from a maDMP standpoint to determine the author of the DMP once everyone has been removed.

plan.owner.name uses a filter on attribute active (see my link above), so it is already nil..