Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove rolify and implement roles within the app #679

Open
mononoken opened this issue May 1, 2024 · 0 comments
Open

Remove rolify and implement roles within the app #679

mononoken opened this issue May 1, 2024 · 0 comments
Labels
on hold Further investigation/decision-making is required

Comments

@mononoken
Copy link
Collaborator

mononoken commented May 1, 2024

If you find any issues with the current roles implementation, please comment here so we can keep track if this tech debt is becoming an issue.

tl;dr

We think that using rolify is unnecessary for this project, and it would be beneficial to build Roles within the app instead. We would want to associate these Roles with organizations via ActsAsTenant so that they are automagically assigned the correct tenant/organization like the rest of the records in the app.

Description

Currently, roles are implemented via the rolify gem. This gem creates a roles table with roles having a name and resource, which is polymorphic. We currently have the resources as Organization records so that each organization has its own set of roles. Roles are associated with Users by the gem using HABTM associations.

[#<Role:***
  id: 1,
  name: "staff",
  resource_type: "Organization",
  resource_id: 1,
  created_at: ***,
  updated_at: ***>,
[#<Role:***
  id: 2,
  name: "staff",
  resource_type: "Organization",
  resource_id: 2, # Same "staff" role name but for a different `Organization`
  created_at: ***,
  updated_at: ***>,
...

We had some discussion about this in the meeting on 2024 April 30. Some points that got brought up:

  • The resource column of roles (which are organizations) is currently not used at all in the app.
    • We considered the possibility that it may not be necessary to associate roles with organizations.
    • However, roles being associated with organizations could be useful if we end up having the option for each organization to have different permissions for their roles (currently, this is not the case).
  • Roles being associated via the rolify defaults feels bad because they are not utilizing ActsAsTenant, so new roles are not automatically associated via the gem. This is inconsistent with the rest of the app, as almost all other models associated with organizations are auto-assigned their tenant by ActsAsTenant.
    • We could keep the rolify Role and just add a new column for organization, which would use ActsAsTenant. However, in that case, the resource column may be completely unused.
    • We could also potentially adjust the current rolify resource association to use the ActsAsTenant helper, but we would need to investigate possible conflicts and what benefits would be gained from this.
  • rolify uses a has_and_belongs_to_many association to relate User and Role. This has limitations as there is no through-table model, so creating validations for the users_roles is not clean.
  • We are not gaining much from using rolify vs just implementing roles ourselves, and it might be causing more confusion than helping since new devs have to look at the gem rather than seeing simple Rails objects.

All that said, we came to the tl;dr conclusion above. However, we thought that since this issue is not pressing, we can hold off on this idea until after MVP or after it does cause any trouble.

Acceptance Criteria

pending

@mononoken mononoken added the on hold Further investigation/decision-making is required label May 1, 2024
@mononoken mononoken changed the title Remove rolify and implement roles with project's requirements Remove rolify and implement roles within the app May 1, 2024
@mononoken mononoken mentioned this issue May 1, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on hold Further investigation/decision-making is required
Projects
None yet
Development

No branches or pull requests

1 participant