djaodjin / djaodjin-saas

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

[WIP] Accessibles page modifications #193

Closed knivets closed 5 years ago

knivets commented 5 years ago

I copied the code from the RoleGrantAcceptView but I'm not sure why we need existing_role check. Considering that when the role grant is created the user is passed already https://github.com/djaodjin/djaodjin-saas/blob/master/saas/api/roles.py#L824.

smirolo commented 5 years ago

The existing_role deals with the case where the user is already a manager for the organization, but now accept a contributor role. It used to be that a user could have multiple roles on an organization but as it turns out, it is best for managing complexity to constraint a user to have one and only one role at a time.

knivets commented 5 years ago

I added a url to extras.py but the problem is that the view that deals with accessibles doesn't inherit from OrganizationMixin so I added the actual url to djaoapp and the one here mostly to keep testsite functioning. We need another solution for global urls.

knivets commented 5 years ago

I basically copy pasted the code from the view that implement the logic, the one you told me about. As for the verification key not being included in the url - the request is not idempotent that’s why it made sense to use POST here.

knivets commented 5 years ago

Also not sure what do you mean by 3rd point in the overall comments list.