[Add]: structure flow to create the assignment new [new pr] (#43)

* Adjust Assignment Table, which should rely on course_to_lms instead of only lms

* Add assignment model

* Implement route of create in assignment controller

* Add course, lms and course_to_lms model for Assignment Controller test

* Add test for create in assignment controller

* Refactor to pass the Codeclimate

* Refactor the controller, by adding intger validation and putting 
ender from helper function to create function

* Fix minor issue based on pr comments

* Add one test to reach a full LOC test coverage

* Add validation check for lms

* Use validation helper

* Update app/helpers/canvas_validation_helper.rb

Co-authored-by: Connor <[email protected]>

* Remove unecessary Rspec

---------

Co-authored-by: Connor <[email protected]>

app/controllers/api/v1/assignments_controller.rb

@@ -1,20 +1,62 @@
 module Api
-    module V1
-      class AssignmentsController < ApplicationController
+  module V1
+    class AssignmentsController < ApplicationController
+      include CanvasValidationHelper
+      before_action :validate_ids!, only: [:create]
 
-        def index
-            render json: 'not yet implemented', status: 501
+      def index
+        render json: { message: 'not yet implemented'} , status: 501
+      end
+
+      # POST /courses/:course_id/lms/:lms_id/assignments
+      def create
+        # Check if the course_to_lms association exists
+        course_to_lms = CourseToLms.find_by(course_id: params[:course_id], lms_id: params[:lms_id])
+
+        unless course_to_lms
+          render json: { error: 'No such Course_LMS association' }, status: :not_found
+          return
         end
 
-        def create
-            render json: 'not yet implemented', status: 501
+        # Check if the assignment already exists
+        if Assignment.exists?(course_to_lms_id: course_to_lms.id, name: params[:name], external_assignment_id: params[:external_assignment_id])
+          render json: { message: 'Record already exists' }, status: :ok
+          return
         end
 
-        def destroy
-            render json: 'not yet implemented', status: 501
+        # Create and render the assignment
+        assignment = Assignment.new(course_to_lms_id: course_to_lms.id, name: params[:name], external_assignment_id: params[:external_assignment_id])
+        if assignment.save
+          render json: assignment, status: :created
+        else
+          render json: assignment.errors, status: :unprocessable_entity
         end
+      end
+
+      def destroy
+        render json: { message: 'not yet implemented'} , status: 501
+      end
 
+      private
+
+
+
+      def validate_ids!
+        begin
+          params.require([:course_id, :lms_id, :name, :external_assignment_id])
+        rescue ActionController::ParameterMissing => e
+          render json: { error: e.message }, status: :bad_request
+          return
+        end
+
+
+        # Validate that course_id and lms_id are integers
+        unless is_valid_course_id(params[:course_id].to_i) && is_valid_lms_id(params[:lms_id].to_i)
+          render json: { error: 'course_id and lms_id must be integers' }, status: :bad_request
+          return
+        end
       end
+
     end
   end
-  
+end

app/helpers/canvas_validation_helper.rb

@@ -10,6 +10,15 @@ def is_valid_course_id(courseId)
     courseId.is_a?(Integer) && courseId > 0
   end
 
+  ##
+  # Checks if the provided lms id is valid.
+  #
+  # @param [Integer] lmsId the lms id to check.
+  # @return [Boolean] whether the provided id is valid.
+  def is_valid_lms_id(lmsId)
+    lmsId.is_a?(Integer) && lmsId > 0
+  end
+
   ##
   # Checks if the provided assignment id is valid.
   #

app/models/assignment.rb

@@ -1,7 +1,8 @@
+# app/models/assignment.rb
 class Assignment < ApplicationRecord
-    #Relationship with Lms
-    belongs_to :lms
-
-    #Relationship with Extension
+    belongs_to :course_to_lms
+
+    validates :name, presence: true
+    validates :external_assignment_id, presence: true
     has_many :extensions
-end
+  end

app/models/course_to_lms.rb

@@ -7,5 +7,4 @@ class CourseToLms < ApplicationRecord
   # Validations
   validates :course_id, presence: true
   validates :lms_id, presence: true
-
-end
+end

db/migrate/20240420232708_change_lms_id_to_course_to_lms_id_in_assignments.rb

@@ -0,0 +1,8 @@
+class ChangeLmsIdToCourseToLmsIdInAssignments < ActiveRecord::Migration[7.1]
+  def change
+    remove_index :assignments, :lms_id
+    remove_column :assignments, :lms_id, :bigint
+    add_column :assignments, :course_to_lms_id, :bigint, null: false
+    add_foreign_key :assignments, :course_to_lmss, column: :course_to_lms_id
+  end
+end

db/schema.rb

@@ -10,17 +10,16 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2024_04_20_211809) do
+ActiveRecord::Schema[7.1].define(version: 2024_04_20_232708) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
   create_table "assignments", force: :cascade do |t|
-    t.bigint "lms_id"
     t.string "name"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     t.string "external_assignment_id"
-    t.index ["lms_id"], name: "index_assignments_on_lms_id"
+    t.bigint "course_to_lms_id", null: false
   end
 
   create_table "course_to_lmss", force: :cascade do |t|
@@ -89,7 +88,7 @@
     t.index ["email"], name: "index_users_on_email", unique: true
   end
 
-  add_foreign_key "assignments", "lmss"
+  add_foreign_key "assignments", "course_to_lmss"
   add_foreign_key "course_to_lmss", "courses"
   add_foreign_key "course_to_lmss", "lmss"
   add_foreign_key "extensions", "assignments"

spec/controllers/api/v1/assignments_controller_spec.rb

@@ -1,28 +1,105 @@
 require 'rails_helper'
 module Api
   module V1
-    RSpec.describe AssignmentsController, type: :request do
-      let(:mock_course_id) { 1 }
-      let(:mock_lms_id) { 1 }
+    describe AssignmentsController do
+      def json_response
+        JSON.parse(response.body)
+      end
+
+      let(:mock_course) { Course.create!(course_name: "Test Course") }
+      let(:mock_lms) { Lms.create!(lms_name: "Test LMS") }
+      let(:mock_course_to_lms) { CourseToLms.create!(course_id: mock_course.id, lms_id: mock_lms.id) }
+
+      let(:valid_params) { { name: "Test Assignment", external_assignment_id: "123ABC", course_id: mock_course.id, lms_id: mock_lms.id } }
+
+      before do
+        mock_course
+        mock_lms
+        mock_course_to_lms
+      end
+
+      after do
+        Assignment.delete_all
+        CourseToLms.delete_all
+        Course.delete_all
+        Lms.delete_all
+      end
 
       describe "GET /api/v1/courses/:course_id/lmss/:lms_id/assignments" do
         it "throws a 501 error" do
-          get "/api/v1/courses/#{mock_course_id}/lmss/#{mock_lms_id}/assignments"
+          get :index, params: { course_id: mock_course.id, lms_id: mock_lms.id }
           expect(response).to have_http_status(501)
         end
       end
 
       describe "POST /api/v1/courses/:course_id/lmss/:lms_id/assignments" do
-        it "throws a 501 error" do
-          post "/api/v1/courses/#{mock_course_id}/lmss/#{mock_lms_id}/assignments"
-          expect(response).to have_http_status(501)
+        context 'when one or more parameters are missing' do
+          it 'returns status :bad_request when name is missing' do
+            post :create, params: valid_params.except(:name)
+            expect(response).to have_http_status(:bad_request)
+            expect(json_response["error"]).to include('param is missing or the value is empty: name')
+          end
+        end
+
+        context 'when course_id or lms_id are not integers' do
+          it 'returns status :bad_request if course_id is not an integer' do
+            post :create, params: valid_params.merge(course_id: 'abc')
+            expect(response).to have_http_status(:bad_request)
+            expect(json_response["error"]).to include('course_id and lms_id must be integers')
+          end
+
+          it 'returns status :bad_request if lms_id is not an integer' do
+            post :create, params: valid_params.merge(lms_id: 'xyz')
+            expect(response).to have_http_status(:bad_request)
+            expect(json_response["error"]).to include('course_id and lms_id must be integers')
+          end
+        end
+
+        context 'save is unsuccessful' do
+          it 'returns status :unprocessable_entity' do
+            allow_any_instance_of(Assignment).to receive(:save).and_return(false)
+            post :create, params: valid_params
+            expect(response).to have_http_status(:unprocessable_entity)
+          end
+        end
+
+        context 'when valid parameters are provided' do
+          it 'creates a new assignment and returns status :created' do
+            post :create, params: valid_params
+            expect(response).to have_http_status(:created)
+            expect(json_response["name"]).to eq(valid_params[:name])
+            expect(json_response["external_assignment_id"]).to eq(valid_params[:external_assignment_id])
+          end
+        end
+
+        context 'when course_to_lms does not exist' do
+          it 'returns status :not_found' do
+            # Ensure this course_to_lms does not exist
+            selected_course = CourseToLms.find_by(course_id: 1, lms_id: 1)
+            selected_course.destroy if selected_course
+            post :create, params: { course_id: 1, lms_id: 1, name: "Test Assignment", external_assignment_id: "123ABC" }
+            expect(response).to have_http_status(:not_found)
+            expect(response.body).to include('No such Course_LMS association')
+          end
         end
+
+        context 'when the assignment already exists' do
+          it 'returns status :ok' do
+            Assignment.create!(course_to_lms_id: mock_course_to_lms.id, name: "Test Assignment", external_assignment_id: "123ABC")
+            post :create, params: valid_params
+            expect(response).to have_http_status(:ok)
+            expect(response.body).to include('Record already exists')
+          end
+        end
+
+
+
       end
 
       describe "DELETE /api/v1/courses/:course_id/lmss/:lms_id/assignments/:id" do
         let(:mock_assignment_id) { 1 }
         it "throws a 501 error" do
-          delete "/api/v1/courses/#{mock_course_id}/lmss/#{mock_lms_id}/assignments/#{mock_assignment_id}"
+          delete :destroy, params: { course_id: mock_course.id, lms_id: mock_lms.id, id: mock_assignment_id }
           expect(response).to have_http_status(501)
         end
       end