How to only let correct_user create or destroy?

93 Views Asked by At

How can we get it where only the correct_user can create or destroy here?

class DaysMissedController < ApplicationController
  before_action :logged_in_user, only: [:create, :destroy]
  before_action :correct_user, only: [:create, :destroy]

  def create
    habit = Habit.find(params[:habit_id])
    habit.missed_days = habit.missed_days + 1
    habit.save!
    level = habit.levels.find(params[:level_id])
    level.missed_days = level.missed_days + 1
    if level.missed_days == 3
      level.missed_days = 0
      level.days_lost += habit.calculate_days_lost + 1
    end
    level.save!
    head :ok
  end

  def destroy
    habit = Habit.find(params[:habit_id])
    habit.missed_days = habit.missed_days - 1
    habit.save
    level = habit.levels.find(params[:level_id])
    level.missed_days = level.missed_days - 1
    level.save!
    head :ok
  end

private

  def correct_user
    @habit = current_user.habits.missed_days.find_by(id: params[:id]) #I've tried different versions of this line
    redirect_to habits_path, notice: "Not authorized to edit this habit" if @habit.nil?
  end
end

Depending on what I put in the line where I commented I either get "Not authorized to edit this habit" or upon refreshing the page the checkmark I had made disappears even if I am the correct_user

habit.js

$(document).ready(function()
{
  $(".habit-check").change(function()
  {
    habit = $(this).parent().siblings(".habit-id").first().attr("id");
    level = $(this).siblings(".level-id").first().attr("id");
    if($(this).is(":checked"))
    {
       $.ajax(
       {
         url: "/habits/" + habit + "/levels/" + level + "/days_missed",
         method: "POST"
       });
    }
    else
    {
       $.ajax(
       {
         url: "/habits/" + habit + "/levels/" + level + "/days_missed/1",
         method: "DELETE"
       });
    }
  });
});

In habit show page this is what I don't want other users to be able to edit via :create & :destroy:

 <% if @habit.current_level_strike %> 
    <div class="btn" id="red"> <label id="<%= @habit.id %>" class="habit-id">Strikes:</label>
  <% else %> 
    <div class="btn" id="gold"> <label id="<%= @habit.id %>" class="habit-id-two">Strikes:</label>
  <% end %>
    <% @habit.levels.each_with_index do |level, index| %>
      <% if @habit.current_level >= (index + 1) %>
        <p>
          <% if @habit.current_level_strike %> 
            <label id="<%= level.id %>" class="level-id">Level <%= index + 1 %>:</label> 
          <% else %> 
            <label id="<%= level.id %>" class="level-id-two">Level <%= index + 1 %>:</label> 
          <% end %>
          <%= check_box_tag nil, true, level.missed_days > 0, {class: "habit-check"} %>
          <%= check_box_tag nil, true, level.missed_days > 1, {class: "habit-check"} %>
          <%= check_box_tag nil, true, level.missed_days > 2, {class: "habit-check"} %>
       </p>
      <% end %>
    <% end %>

rails console

[2] pry(main)> Habit.find(1)
  Habit Load (17.7ms)  SELECT  "habits".* FROM "habits" WHERE "habits"."id" = ? LIMIT 1  [["id", 1]]
=> #<Habit:0x007f96c7133728
 id: 1,
 conceal: false,
 missed_days: 0,
 likes: 1,
 committed: ["mon", "tue", "wed", "thu", "fri", ""],
 date_started: Sun, 07 Jun 2015 00:00:00 UTC +00:00,
 trigger: "",
 action: "test",
 target: "",
 reward: "",
 private_submit: nil,
 user_id: 1,
 created_at: Sun, 07 Jun 2015 21:57:04 UTC +00:00,
 updated_at: Wed, 10 Jun 2015 00:45:05 UTC +00:00,
 order: nil>
[3] pry(main)> Level.find(1)
  Level Load (14.0ms)  SELECT  "levels".* FROM "levels" WHERE "levels"."id" = ? LIMIT 1  [["id", 1]]
=> #<Level:0x007f96c7838c80
 id: 1,
 habit_id: 1,
 missed_days: 0,
 current_level: nil,
 created_at: Sun, 07 Jun 2015 21:57:04 UTC +00:00,
 updated_at: Wed, 10 Jun 2015 00:45:05 UTC +00:00,
 days_lost: 0>
[4] pry(main)>

habits_controller

class HabitsController < ApplicationController
  before_action :set_habit, only: [:show, :edit, :update, :destroy, :like]
  before_action :logged_in_user, only: [:create, :destroy]
  before_action :correct_user, only: [:edit, :update, :destroy]

  def index
    if params[:tag]
      @habits = Habit.tagged_with(params[:tag])
    else
      @habits = current_user.habits.order("date_started DESC")
    end
  end

  def show
    @habit = Habit.find(params[:id])
    @notable = @habit
    @notes = @notable.notes
    @note = Note.new
    @commentable = @habit
    @comments = @commentable.comments
    @comment = Comment.new
    @correct_user = current_user.habits.find_by(id: params[:id])
  end

  def new
    @habit = current_user.habits.build
  end

  def edit
  end

  def create
    @habit = current_user.habits.build(habit_params)
    if (params[:commit] == 'conceal')
      @habit.conceal = true
      @habit.save_with_current_level
      redirect_to @habit, notice: 'Habit was successfully created'
    elsif
      @habit.save_with_current_level
      track_activity @habit
      redirect_to @habit, notice: 'Habit was successfully created'
    else
      flash.now[:danger] = 'Required Fields: "Committed to", "Started", and "Enter Habit"'
      render 'new'
    end
  end

  def update
    if @habit.update(habit_params)
      redirect_to @habit, notice: 'Habit was successfully updated.'
    else
      render action: 'edit'
    end
  end

  def destroy
    @habit.destroy
    redirect_to habits_url
  end

  def like
    @habit = Habit.find(params[:id])
    @habit_like = current_user.habit_likes.build(habit: @habit)
    if @habit_like.save
      @habit.increment!(:likes)
      flash[:success] = 'Thanks for liking!'
    else
      flash[:error] = 'Two many likes'
    end  
      redirect_to(:back)
  end

  private
    def set_habit
      @habit = Habit.find(params[:id])
    end

    def correct_user
      @habit = current_user.habits.find_by(id: params[:id])
      redirect_to habits_path, notice: "Not authorized to edit this habit" if @habit.nil?
    end

  def habit_params
    params.require(:habit).permit(
      :user_id, 
      :trigger,
      :tag_list,
      :current_level,
      :missed_days,
      :target, 
      :reward,
      :comment,
      :commentable,
      :like,
      :likeable,
      :private,
      :action,
      :order,
      :date_started,
      :missed_one,
      :notes_text,
      :notes_date, 
      :notable, 
      :note, 
      :committed => [],
      levels_attributes: [
      :missed_days,
      :days_lost], notes_attributes: [:notable, :note, :notes_text, :notes_date, :_destroy])
  end
end

Here's the gist of it (not fully updated but you get the idea)

Please let me know if you need further explanation or code to help you help me :)

UPDATE ERROR

Started POST "/habits/1/levels/2/days_missed" for 127.0.0.1 at 2015-06-10 13:07:11 -0400
Processing by DaysMissedController#create as */*
  Parameters: {"habit_id"=>"1", "level_id"=>"2"}
  User Load (0.2ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = ? LIMIT 1  [["id", 1]]
  Habit Load (0.2ms)  SELECT "habits".* FROM "habits" WHERE "habits"."user_id" = ?  [["user_id", 1]]
  ActsAsTaggableOn::Tag Load (0.2ms)  SELECT "tags".* FROM "tags" WHERE (LOWER(name) = LOWER('ingrain'))
  CACHE (0.0ms)  SELECT "habits".* FROM "habits" WHERE "habits"."user_id" = ?  [["user_id", 1]]
  Level Load (0.2ms)  SELECT "levels".* FROM "levels" WHERE "levels"."habit_id" = ?  [["habit_id", 1]]
   (0.2ms)  SELECT COUNT(*) FROM "habits" WHERE "habits"."user_id" = ?  [["user_id", 1]]
Completed 404 Not Found in 21ms

ActiveRecord::RecordNotFound (Couldn't find Habit without an ID):
  app/controllers/days_missed_controller.rb:32:in `correct_user'
1

There are 1 best solutions below

3
On BEST ANSWER

So I believe you've correctly identified that the problem lies within your correct_user method:

def correct_user
  @habit = current_user.habits.missed_days.find_by(id: params[:id])
  redirect_to habits_path, notice: "Not authorized to edit this habit" if @habit.nil?
end

Take a look at your first line, here's what I read in English:

  • Take the current User
  • Get that User's Habits
  • Get those Habits' MissedDays
  • Find me one of those missed days that matches params[:id]

Which, based on your data model, is sort of non-sensical. Missed Days is an attribute of a model, not a model itself. I'm not sure you can even call .find_by on a number, which makes it unsurprising that you keep getting nil.

I bet this'll work instead:

@habit = current_user.habits.find(params[:habit_id])

Note that the find method always queries by the id attribute. Saved you a few characters. :)

One other thing, in both create and destroy, you re-find the habit that you already found in the before_action. Try this:

def create
  @habit.missed_days = habit.missed_days + 1
  @habit.save
  ...
end