CUCentralAdvancement / giving-backend

Home of CMS that powers giving.cu.edu amongst other Central Advancement content needs.
MIT License
1 stars 0 forks source link

Refactor Space/User Relationship To Add Membership #91

Closed alexfinnarn closed 3 years ago

alexfinnarn commented 3 years ago

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, https://github.com/CUCentralAdvancement/giving-backend/blob/67da67035323103c739051181f8aca0226acf87d/config/routes.rb#L7, 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

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.

alexfinnarn commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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:

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 commented 3 years ago

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.