2

In our Rails app, we have a CalendarsController:

class CalendarsController < ApplicationController

  def create
    @calendar = current_user.calendars.create(calendar_params)
    current_user.add_calendar_and_role(@calendar.id, 'Owner')
    if @calendar.save
      current_user.total_calendar_count += 1
      current_user.owned_calendar_count += 1
      current_user.save
      flash[:success] = "Calendar created!"
      redirect_to dashboard_path
    else
      render 'static_pages/home'
    end
  end

  def show
    @calendar = Calendar.find(params[:id])
    @posts = @calendar.posts
    @post = Post.new
  end

  def index
  end

  def edit
  end

  def destroy
    Calendar.find(params[:id]).destroy
    flash[:success] = "Calendar deleted"
    redirect_to dashboard_path
  end

  private

    def calendar_params
      params.require(:calendar).permit(:name)
    end

end

In the create action, when a new @calendar is created, we run @calendar.save to check if the new instance has actually been created, and then perform some actions.

We would like to implement a similar process in our destroy action.

We are thinking of updating the destroy method as follows:

def destroy
  @calendar = Calendar.find(params[:id])
  @calendar.destroy
  if @calendar.delete
    flash[:success] = "Calendar deleted"
    current_user.total_calendar_count -= 1
    if @calendar.administrations.role == "Owner"
      current_user.owned_calendar_count -= 1
    end
  end
  redirect_to dashboard_path
end

Is the syntax of this code correct, in particular if @calendar.delete and if @calendar.administrations.role == "Owner"?

And, most importantly, would the code of this destroy action make sense?

Fran Martinez
  • 2,994
  • 2
  • 26
  • 38
Thibaud Clement
  • 6,607
  • 10
  • 50
  • 103

2 Answers2

3

Did you think about using the persisted? method

@calendar.destroy
unless @calendar.persisted?
   ... some code here ....
end
Fran Martinez
  • 2,994
  • 2
  • 26
  • 38
  • No, I did not. What would be the advantage of the other code structure? Is this the Rails way to do it? – Thibaud Clement Jul 09 '15 at 16:37
  • persisted? checks if your record is still in the database or just in memory. If it's not "persisted" in the database, it will return "false". Is what you want to do. It's a Rails ActiveRecord method: http://apidock.com/rails/ActiveRecord/Persistence/persisted%3F – Fran Martinez Jul 09 '15 at 16:39
  • That does sound like what we want to achieve, thank you very much. – Thibaud Clement Jul 09 '15 at 17:02
  • no problem! please check my answer as the correct one so I will get some magic power :D – Fran Martinez Jul 10 '15 at 06:22
1

I believe it would be more like:

def destroy
  @calendar = Calendar.find(params[:id])
  calendar_admin_role = @calendar.administrations.role
  if @calendar.destroy
    flash[:success] = "Calendar deleted"
    current_user.total_calendar_count -= 1
    if calendar_admin_role == "Owner"
      current_user.owned_calendar_count -= 1
    end
  end
  redirect_to dashboard_path
end

But this is off the top of my head after a long day at work so could be wrong.

James Smith
  • 501
  • 5
  • 13
  • Ok thanks. Just for my information, why in the `action` method, do we use `if @calendar.save` instead of ``if @calendar.create`, but in the `destroy` method, we do use `if @calendar.destroy` and not `if @calendar.delete`. Sorry if this is a dumb question, I am pretty new to Ruby & Rails, and I am a bit confused about that. – Thibaud Clement Jul 09 '15 at 16:36
  • 1
    In this instance it might be more "Rails" to have `@calendar = Calendar.build(calendar_params)`, then later `if @calendar.save`. `build` doesn't create anything in the DB, `save` does that. `delete` is different from `destroy`. It removes objects without running their callbacks. See http://stackoverflow.com/questions/22757450/difference-between-destroy-and-delete – James Smith Jul 10 '15 at 09:42