Controller for classrooms and students

Question

The actions I'm wondering about are members, add_student, and remove_student. They work just fine, but they aren't RESTful and it feels like they should be handled (perhaps) by the StudentsController.

Note that members and show are the same. The only difference is that the members.html.erb view iterates over and displays @classroom.students while show.html.erb displays info about the classroom itself.

class ClassroomsController < ApplicationController
 # GET /classrooms
 def index
 @classrooms = Classroom.all.order('name ASC')
 end
 # GET /classrooms/1
 def show
 @classroom = Classroom.find(params[:id])
 authorize @classroom
 end
 # GET /classrooms/1/members
 def members
 @classroom = Classroom.find(params[:id])
 authorize @classroom
 end
 # GET /classrooms/new
 def new
 @classroom = Classroom.new
 @classroom.build_avatar
 authorize @classroom
 end
 # GET /classrooms/1/edit
 def edit
 @classroom = Classroom.find(params[:id])
 @classroom.build_avatar unless @classroom.avatar
 authorize @classroom
 end
 # POST /classrooms
 def create
 @classroom = Classroom.new(classroom_params)
 authorize @classroom
 if @classroom.save
 redirect_to classrooms_path, notice: 'Classroom was successfully created.'
 else
 render action: "new"
 end
 end
 # PUT /classrooms/1
 def update
 @classroom = Classroom.find(params[:id])
 authorize @classroom
 if @classroom.update(classroom_params)
 redirect_to edit_classroom_path(@classroom), notice: 'Classroom was successfully updated.'
 else
 render action: "edit"
 end
 end
 def add_student
 @classroom = Classroom.find(params[:id])
 @student = Student.find(params[:classroom][:student_ids][0])
 authorize @classroom
 @classroom.students << @student
 respond_to do |format|
 @count = @classroom.students.count
 format.js
 end
 end
 def remove_student
 @classroom = Classroom.find(params[:id])
 @student = Student.find(params[:classroom][:student_ids][0])
 authorize @classroom
 @classroom.students.delete(@student)
 respond_to do |format|
 @count = @classroom.students.count
 format.js
 end
 end
 # DELETE /classrooms/1
 def destroy
 @classroom = Classroom.find(params[:id])
 authorize @classroom
 @classroom.destroy
 redirect_to classrooms_path
 end
 private
 def classroom_params
 params.require(:classroom).permit(:name, :description, avatar_attributes: [:id, :avatar, :_destroy],
 :student_ids => [])
 end
end

Here are the models:

# student.rb
class Student < ActiveRecord::Base
 has_many :classroom_memberships, :dependent => :destroy, :inverse_of => :student
 has_many :classrooms, :through => :classroom_memberships
end
# classroom.rb
class Classroom < ActiveRecord::Base
 has_many :classroom_memberships, :inverse_of => :classroom, :dependent => :destroy
 has_many :munchkins, :through => :classroom_memberships
end

Answer 1

The actions I'm wondering about are members, add_student, and remove_student. They work just fine, but they aren't RESTful [...]

True, they're not part the usual CRUD actions that can be expressed with HTTP verbs. But then neither are the default new or edit actions, so it's not like going beyond CRUD is automatically illegal.

Even so, associations do always present a bit of challenge. Are you adding the student to the class, or the class to the student?

However, in this case it appears that you already have a join model: ClassroomMembership. So going back to the CRUD actions, consider what you're creating when you call add_student: A membership.

So what you might want to do is simply make a ClassroomMembershipsController, since that's the resource you're manipulating. Said controller can then have the usual RESTful CRUD actions, keeping StudentsController or ClassroomController pretty clean.

This is just an idea, though. Again, if it's simpler to keep your actions on the ClassroomController then do that. But conceptually what you're doing is manipulating the association model.

• Rob Sobers: Adding a ClassroomMembershipsController makes a lot of sense! Definitely a more RESTful approach. Follow-up question: ClassroomMemberships don't have their own views--so I guess I just render the Classroom view from the ClassroomMembershipsController and that's kosher?
• Flambino: @RobSobers Or you can just do a redirect_to @membership.classroom. A controller doesn't need to have views; its purpose is merely to respond to requests, and sending a redirect is a response. The default destroy action doesn't have a view either, since it just redirects. Incidentally, I'd consider the name Enrollment for the join model, only because it sounds a little more straightforward. But renaming a resource is a chore and has knock-on effect, so don't do it lightly. It can be done of course, just know what you're getting yourself into :)
• Rob Sobers: Good point. And I suppose in this case, I won't even need to redirect since both add_student and remove_student respond with JavaScript, which will be sent to whichever page is making the request.
• Flambino: @RobSobers Sure. In that case, I often find it easiest to respond with plain HTTP status codes, and no actual content. E.g. instead of render ... or redirect_to ..., you can just call head :created (when a membership was created), head :conflict if the membership already exists, etc. etc.. See httpstatus.es for more. There are plenty of standardised response codes, that carry a lot of meaning

Answer 2

You could refactor add_student and remove_student using blocks

 def add_student
 find_class_and_student do
 @classroom.students << @student 
 end
 end
 def remove_student
 find_class_and_student do
 @classroom.students.delete(@student)
 end
 end
 private
 def find_class_and_student
 @classroom = Classroom.find(params[:id])
 @student = Student.find(params[:classroom][:student_ids][0]) 
 authorize @classroom
 yield
 respond_to do |format|
 @count = @classroom.students.count
 format.js
 end 
 end

This is just pulling your code inside some other actions.