Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Refactor Space/User Relationship To Add Membership #91

Closed
4 of 7 tasks
alexfinnarn opened this issue Jul 1, 2021 · 4 comments
Closed
4 of 7 tasks

Refactor Space/User Relationship To Add Membership #91

alexfinnarn opened this issue Jul 1, 2021 · 4 comments
Assignees

Comments

@alexfinnarn
Copy link
Contributor

alexfinnarn commented Jul 1, 2021

In a developer chat, someone said "I have never seen a good argument for using HABTM" which means a has_and_belongs_to_many relationship, which I added between the Space and User models.

However, the real relationship is a "Membership" which can have many properties like type, start date, end date, etc...I thought of a gym for some reason and how I'm a member through a membership, which holds a lot of properties about the relationship.

The simplest rule of thumb is that you should set up a has_many :through relationship if you need to work with the relationship model as an independent entity.

So now I will add a Membership model that can be accessed via /spaces/[space-slug]/members. I already set up an empty route at that place,

get 'members' => 'spaces#members'
, but now it will be a whole resource instead of just a controller action.

This actually fits more into RESTful concepts where everything uses CRUD operations and no other verbs. Any action that doesn't fit with updating the state of the current resource is probably better off being a nested resource with its own CRUD endpoints.

Acceptance Criteria

  • Membership model created tieing Space and User together; fields of type:string start_date:datetime end_date:datetime
  • Membership resource nested under /spaces/[space-slug]/members
  • Relationship to Space and User declared via Active Record syntax
  • Policy created with Pundit
  • Add state machine to Membership to handle membership status #98
  • Add Cypress tests for space functionality of adding and deleting user memberships.

Next Issue

Memberships need to be more fleshed out. I can think of some properties to start out, but they are just placeholders that seem useful.

  • Implement state machine with the Fund model.
@alexfinnarn
Copy link
Contributor Author

I never thought this would come up, but when I started adding the code for "type" of membership, I ended up adding a state machine to the Membership model.

Originally, I was going to go with an enum and map integers to the basic membership types as I did with Faqs. However, the Faq categories came from Drupal taxonomy term IDs and fit well to that model.

In looking up more about enum support in Rails, I came to a project that placed enums in classes rather than a database lookup. This seemed like an improvement to me since the membership types would be few in number, but the author of the gem mentioned state machines as something more complex/complete when their gem wasn't enough.

So, in this case, we can have a state machine for the membership status and an enum for the membership type. I did find https://github.com/beerlington/classy_enum for a better way to describe the enum values and their behavior, but I'm not sure that is needed now and it is not maintained.

@alexfinnarn
Copy link
Contributor Author

I added memberships via a method on the Spaces controller that links to a plain resources :memberships instead of trying to nest anything.

I suppose it would be good to allow users to add multiple people to spaces at a time, but that can be another issue. There were/are many pieces that I half-tried to improve before I realized I was getting ahead of myself.

@alexfinnarn
Copy link
Contributor Author

https://www.youtube.com/watch?v=HctYHe-YjnE

That talk is great about how to think more RESTfully in Rails. I remembered seeing it before, but I found it again in a recent dev Slack thread.

The parts that stuck out to most this time were:

  • Using a state machine in Rails can get dense and hard to read the more events and transitions you include. The presenter has an example of a state machine he turns into RESTful resources and simplifies the code.
  • Custom controller actions are usually bad news. I added a /spaces/[:space]/members action, then I changed it to a nested resource, and then I changed it back to a custom controller action as I stumbled around figuring more things out in Rails and wanting to close this issue. I now think memberships should have a controller for working with spaces, something like a SpaceMemberships controller, but I need to think more about it. If the controller for /memberships can work the same as /spaces/[:space]/members, then it can be a resource with only the index action. I don't see why you need to edit a membership under the /spaces namespace...well except for permissions.
  • Controllers and models should be boring with the use of query and command objects. This comes from DDD, which would be a good book to read further into.

I don't think I will refactor anything right now so I can move on to UI-related issues like the WYSIWYG and adding layouts to pages.

@alexfinnarn
Copy link
Contributor Author

The tests pass locally, but due to Webpacker sucking and taking time to compile assets on the first test run causes that test to fail. I can think of ways to "fix" this, but also I think this problem won't exist if I try to use another solution like Vite Ruby.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant