djaodjin / djaodjin-saas

Django application for software-as-service and subscription businesses
Other
564 stars 124 forks source link

Merge managers and contributors pages into a roles page #90

Closed smirolo closed 8 years ago

smirolo commented 8 years ago

It is currently possible to have generic roles in the database but the views and API are limited to managers and contributors. Those roles views / API should be updated to support generic roles.

smirolo commented 8 years ago

When a User requests a Role on an Organization, a manager of the Organization will decide which permissions (RoleDescription) to give that user. Thus we do not have a request for a specific role.

There are two ways to generate requests then. Either a Role record is inserted with an inactive tag (i.e. request_key is not null) or a RequestRole table must be created.

I prefer the first approach because it integrates smoothly with already deployed code. Hence a Role is only modified as such (see role-descriptor branch):

class Role(models.Model):
....
-     name = models.CharField(max_length=20)
+    role_description = models.ForeignKey(RoleDescription, null=True)

We just replace the name field by a role_description foreign key. If we were to do select distinct(name) from saas_role; role names would disappear when the last Role with that name is removed. Because RoleDescription are actual records, the foreign key approach handles the case of a RoleDescription is defined but they are zero Role using it.

To replicate the current setup, it is only necessary to define two RoleDescription objects and update the saas_role table accordingly. i.e.:

$ INSERT INTO saas_roledescription (name) VALUES ('manager'), ('contributor');
$ UPDATE saas_role SET role_description = 1 WHERE name='manager';
$ UPDATE saas_role SET role_description = 2 WHERE name='contributor';

The organization foreign key in RoleDescription could be null. When it is the RoleDescription definition is valid across the whole site (ex: manager/contributor). When the organization foreign key in RoleDescription is not null, it means the RoleDescription is defined in the context of a specific organization. The other way to resolve conflicts between two organizations with a different definition of RoleDescription named "support_staff" would be to impose RoleDescription.slug to be unique across the whole database.

In all scenarios, there must be at least one User who is manager of an Organization otherwise the Organization is "dead", i.e. With no User connected to an Organization through a Role, there is no possibility to modify that Organization profile/subscriptions/etc. short of accessing the database directly through a SQL shell.

The organization field in RoleDescription is used to create unique identities in case two Organization define a RoleDescription with the same name. It is not meant to retrieve the organization participating in a User <- through Role -> Organization relationship. The organization field in Role is doing that. RoleDescription is only a qualifier of the Role but the role still exists even when Role.role_description is null.

smirolo commented 8 years ago

We need to keep the RoleDetailView view while the RoleListView is implemented to support multiple roles. That way we can deploy changes to the database independently of the UI.

I recommend to rename templates/saas/profile/roles.html to templates/saas/profile/role_detail.html and write a new roles.html matching multiple RoleDescription shown on the page.

Either the roles.html templates is using a list of RoleDescription passed in the template context, or the various RoleDescription columns on the page are added through AngularJS (and an API call to saas_api_role_description_list). No preference. First approach might be better from a load time perspective. Second approach might be less messy to implement the "Add RoleDescription" functionality.

In both cases, the simplest is to generate API requests on saas_api_role_filtered_list (one request per RoleDescription). A single API request to saas_api_role_list and populating the various columns in the AngularJS controller would surely be more efficient but it is OK to optimize for developer productivity rather than system performance at this point.

smirolo commented 8 years ago

See pull request #100 and pull request #102.