Merge pull request #63 from WWC-Hackathon-2023/1refactor

BE | Refactor `PuzzlesController#index` & Cleanup `UserPuzzlesController`

app/controllers/api/v1/puzzles_controller.rb

@@ -2,15 +2,24 @@ class Api::V1::PuzzlesController < ApplicationController
   skip_before_action :set_current_user, only: [:index]
 
   def index
-    zip_code = params[:zip_code]
-    users = User.where(zip_code:) if zip_code.present?
+    find_puzzles(params[:zip_code])
+    raise NoPuzzlesException if @puzzles_in_zip_code.empty?
+    render json: PuzzleSerializer.new(@puzzles_in_zip_code)
+  end
+
+  private 
 
-    if users != []
-      puzzles = Puzzle.where(user_id: users.pluck(:id))
-      current_puzzles = puzzles.where.not(status: 3)
-      render json: PuzzleSerializer.new(current_puzzles)
-    elsif puzzles == [] || users == []
-      render json: { error: "Puzzles not found in this area" }, status: "404"
-    end
+  def find_puzzles(zip_code)
+    @puzzles_in_zip_code = Puzzle.find_by_zip_code(zip_code)
   end
 end
+
+# Note to self: Saving this to remember process that lead me to final version:
+#def index
+  # puzzles_in_zip_code = Puzzle.find_by_zip_code(params[:zip_code])
+  # if @puzzles_in_zip_code.any?
+  #   render json: PuzzleSerializer.new(@puzzles_in_zip_code)
+  # else
+  #   raise NoPuzzlesException
+  # end
+# end

app/controllers/api/v1/users/puzzles_controller.rb

@@ -1,34 +1,37 @@
 class Api::V1::Users::PuzzlesController < ApplicationController
   before_action :find_user
+  before_action :find_puzzle, only: [:update]
 
   def index
     render json: PuzzleSerializer.new(@user.puzzles)
   end
 
   def show
-    puzzle = @user.puzzles.find(params[:puzzle_id])
-    render json: PuzzleSerializer.new(puzzle)
+    show_puzzle = @user.puzzles.find(params[:puzzle_id])
+    render json: PuzzleSerializer.new(show_puzzle)
   end
 
   def create
-    puzzle = @user.puzzles.new(puzzle_params)
-    render json: PuzzleSerializer.new(puzzle), status: 201 if puzzle.save
+    new_puzzle = @user.puzzles.new(puzzle_params)
+    render json: PuzzleSerializer.new(new_puzzle), status: 201 if new_puzzle.save
   end
 
   def update
-    puzzle = Puzzle.find(params[:puzzle_id])
-
-    puzzle.update(puzzle_params)
-    render json: PuzzleSerializer.new(puzzle)
+    @puzzle.update(puzzle_params)
+    render json: PuzzleSerializer.new(@puzzle)
   end
 
   private
 
   def puzzle_params
-    params.permit(:status, :title, :description, :total_pieces, :notes, :puzzle_image_url) # did not include user_id
+    params.permit(:status, :title, :description, :total_pieces, :notes, :puzzle_image_url) # did not include user_id since this will not be updated & is available when created already
   end
 
   def find_user
     @user = User.find(params[:user_id])
   end
+
+  def find_puzzle
+    @puzzle = Puzzle.find(params[:puzzle_id])
+  end
 end

app/controllers/api/v1/users_controller.rb

@@ -1,26 +1,31 @@
 class Api::V1::UsersController < ApplicationController
+  before_action :find_user, only: [:show, :dashboard]
+
   def show
-    render json: UserSerializer.new(User.find(params[:user_id]))
+    render json: UserSerializer.new(@user)
   end
 
   def dashboard
-    dashboard = User.find(params[:user_id]).find_dashboard_info
+    dashboard = @user.find_dashboard_info
     render json: DashboardSerializer.new(dashboard)
   end
 
   def create
     new_user = User.new(user_params)
-    new_user.email.downcase
-    new_user.format_phone_number
-    return unless new_user.save
-
-    session[:user_id] = new_user.id
-    render json: UserSerializer.new(new_user), status: :created
+    new_user.format_attributes
+    if new_user.save
+      session[:user_id] = new_user.id
+      render json: UserSerializer.new(new_user), status: :created
+    end
   end
 
   private
 
   def user_params
     params.permit(:full_name, :password, :password_confirmation, :email, :zip_code, :phone_number)
   end
+
+  def find_user
+    @user = User.find(params[:user_id])
+  end
 end

app/controllers/application_controller.rb

@@ -3,12 +3,24 @@
 class ApplicationController < ActionController::API
   before_action :set_current_user
   rescue_from ActiveRecord::RecordNotFound, with: :record_not_found
+  rescue_from NoPuzzlesException, with: :no_puzzles
 
   def record_not_found(exception)
     render json: ErrorSerializer.new(exception, 404).serializable_hash, status: :not_found # 404
   end
 
+  def no_puzzles
+  error = NoPuzzlesException.new("No puzzles found in this area.")
+  render json: ErrorSerializer.new(error, :not_found).serializable_hash, status: :not_found # 404
+  end
+
   def set_current_user
     @current_user ||= User.find_by(id: session[:user_id])
   end
 end
+
+# Note to self: Saving this to remember process that lead me to final version:
+# def no_puzzles
+  # render status: :no_content, json: {} # 204
+  # render json: { error: "No puzzles found in this area." }, status: :not_found # 404
+# end

app/exceptions/no_puzzles_exception.rb

@@ -0,0 +1,7 @@
+class NoPuzzlesException < StandardError
+end
+
+
+
+
+

app/models/puzzle.rb

@@ -6,4 +6,10 @@ class Puzzle < ApplicationRecord
   validates_numericality_of :total_pieces
 
   enum status: { 'Available' => 0, 'Pending' => 1, 'Not Available' => 2, "Permanently Removed" => 3 }
+
+  def self.find_by_zip_code(zip_code)
+    joins(:user)
+      .where(users: { zip_code: zip_code })
+      .where.not(status: 3)
+  end
 end

app/models/user.rb

@@ -11,8 +11,9 @@ class User < ApplicationRecord
 
   has_secure_password
 
-  def format_phone_number
-    phone_number.insert(0, '(').insert(4, ')').insert(5, " ").insert(9, "-")
+  def format_attributes
+    self.email = format_email
+    self.phone_number = format_phone_number
   end
 
   # This method needs a HUGE refactor:
@@ -64,4 +65,14 @@ def find_dashboard_info
 
     OpenStruct.new(dashboard_info)
   end
+
+  private
+
+  def format_email
+    email.downcase
+  end
+
+  def format_phone_number
+    phone_number.insert(0, '(').insert(4, ')').insert(5, " ").insert(9, "-")
+  end
 end

spec/models/puzzle_spec.rb

@@ -17,4 +17,44 @@
 
     it { should define_enum_for(:status).with_values(["Available", "Pending", "Not Available", "Permanently Removed"]) }
   end
+
+  describe "class methods" do
+    describe "::find_by_zip_code" do
+      before(:each) do
+        @user_1 = create(:user, id: 1, zip_code: 12345)
+        @user_2 = create(:user, id: 2, zip_code: 12345)
+        @user_3 = create(:user, id: 3, zip_code: 54321)
+
+        @puzzle_1 = create(:puzzle, user: @user_1)
+        @puzzle_2 = create(:puzzle, user: @user_1)
+
+        @puzzle_3 = create(:puzzle, user: @user_2)
+        @puzzle_4 = create(:puzzle, user: @user_2)
+
+        @puzzle_5 = create(:puzzle, user: @user_3)
+        @puzzle_6 = create(:puzzle, user: @user_3)
+      end
+
+      it "returns all puzzles within a zip code" do
+        puzzles_in_zip_code = Puzzle.find_by_zip_code(12345)
+
+        expect(puzzles_in_zip_code).to eq([@puzzle_1, @puzzle_2, @puzzle_3, @puzzle_4])
+        expect(puzzles_in_zip_code).to_not include([@puzzle_5, @puzzle_6])
+      end
+
+      it "returns an empty array if no puzzles are found in a zip code" do
+        puzzles_in_zip_code = Puzzle.find_by_zip_code(10101)
+
+        expect(puzzles_in_zip_code).to eq([])
+        expect(puzzles_in_zip_code).to_not include([@puzzle_1, @puzzle_2, @puzzle_3, @puzzle_4, @puzzle_5, @puzzle_6])
+      end
+
+      it "returns an empty array if nil is sent in place of a zip code" do
+        puzzles_in_zip_code = Puzzle.find_by_zip_code(nil)
+
+        expect(puzzles_in_zip_code).to eq([])
+        expect(puzzles_in_zip_code).to_not include([@puzzle_1, @puzzle_2, @puzzle_3, @puzzle_4, @puzzle_5, @puzzle_6])
+      end
+    end
+  end
 end

spec/models/user_spec.rb

@@ -18,6 +18,24 @@
   end
 
   describe "instance methods" do
+    describe "#format_attributes" do
+      it "can format the email & phone number attributes of a user" do
+        user = User.create(
+          full_name: "Diana Puzzler",
+          password: "PuzzleQueen1",
+          password_confirmation: "PuzzleQueen1",
+          email: "[email protected]",
+          zip_code: 12345,
+          phone_number: "1011110000"
+        )
+
+        user.format_attributes
+
+        expect(user.email).to eq("[email protected]")
+        expect(user.phone_number).to eq("(101) 111-0000")
+      end
+    end
+
     describe "#find_dashboard_info" do
       before(:each) do
         @user_1 = create(:user, id: 1)