From 16f6a269a528deca444c423a1be857f1e84a1c99 Mon Sep 17 00:00:00 2001 From: Zzz212zzZ <2120020201@qq.com> Date: Thu, 18 Apr 2024 22:30:50 -0700 Subject: [PATCH 01/12] Add models for course_to_lms --- app/models/course.rb | 6 ++++++ app/models/course_to_lms.rb | 11 +++++++++++ app/models/lms.rb | 5 +++++ 3 files changed, 22 insertions(+) create mode 100644 app/models/course.rb create mode 100644 app/models/course_to_lms.rb create mode 100644 app/models/lms.rb diff --git a/app/models/course.rb b/app/models/course.rb new file mode 100644 index 0000000..200c66e --- /dev/null +++ b/app/models/course.rb @@ -0,0 +1,6 @@ +# app/models/course.rb + +class Course < ApplicationRecord + + +end \ No newline at end of file diff --git a/app/models/course_to_lms.rb b/app/models/course_to_lms.rb new file mode 100644 index 0000000..70c3f04 --- /dev/null +++ b/app/models/course_to_lms.rb @@ -0,0 +1,11 @@ +# app/models/course_to_lms.rb +class CourseToLms < ApplicationRecord + # Association with Course and Lms + belongs_to :course + belongs_to :lms + + # You can add validations here if needed, for example: + validates :course_id, presence: true + validates :lms_id, presence: true + end + \ No newline at end of file diff --git a/app/models/lms.rb b/app/models/lms.rb new file mode 100644 index 0000000..6f24f6f --- /dev/null +++ b/app/models/lms.rb @@ -0,0 +1,5 @@ +# app/models/lms.rb +class Lms < ApplicationRecord + + +end \ No newline at end of file From f48fa291a9c29ebe8b9de7a6f39f0b40745e15a5 Mon Sep 17 00:00:00 2001 From: Zzz212zzZ <2120020201@qq.com> Date: Thu, 18 Apr 2024 22:31:29 -0700 Subject: [PATCH 02/12] Implement creating association in course_to_lms --- app/controllers/api/v1/lmss_controller.rb | 43 ++++++++++++++++++++--- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/app/controllers/api/v1/lmss_controller.rb b/app/controllers/api/v1/lmss_controller.rb index 499af77..cb15555 100644 --- a/app/controllers/api/v1/lmss_controller.rb +++ b/app/controllers/api/v1/lmss_controller.rb @@ -1,10 +1,43 @@ module Api module V1 class LmssController < BaseController - before_action :validate_name!, only: [:create] + before_action :validate_ids!, only: [:create] + # POST /courses/:course_id/lmss def create - render :json => 'not yet implemented'.to_json, status: 501 + course_id = params[:course_id] + lms_id = params[:lms_id] + + # Ensure the course exists + unless Course.exists?(course_id) + render json: { error: 'Course not found' }, status: :not_found + return + end + + # Ensure the lms exists + unless Lms.exists?(lms_id) + render json: { error: 'Lms not found' }, status: :not_found + return + end + + # Check if the entry already exists + existing_entry = CourseToLms.find_by(course_id: course_id, lms_id: lms_id) + if existing_entry + render json: { message: 'The association between the specified course and LMS already exists.' }, status: :ok + return + end + + # Create the course_to_lms entry + course_to_lms = CourseToLms.new( + course_id: course_id, + lms_id: lms_id + ) + + if course_to_lms.save + render json: course_to_lms, status: :created + else + render json: course_to_lms.errors, status: :unprocessable_entity + end end def index @@ -20,9 +53,9 @@ def destroy # TODO: this should be exported to its own (validator) class. # TODO: this validation should also check the config file for the name of lms's. # - def validate_name! - if params['name'].blank? - render :json => 'name parameter is required', status: 401 + def validate_ids! + if params[:course_id].blank? || params[:lms_id].blank? + render json: { error: 'course_id and lms_id are required' }, status: :bad_request end end end From bb98d3d805dca8a7bd6f898388873d1985cd5b64 Mon Sep 17 00:00:00 2001 From: Zzz212zzZ <2120020201@qq.com> Date: Thu, 18 Apr 2024 22:31:53 -0700 Subject: [PATCH 03/12] Test for creating association in course_to_lms --- .../api/v1/lmss_controller_spec.rb | 74 +++++++++++++++---- 1 file changed, 59 insertions(+), 15 deletions(-) diff --git a/spec/controllers/api/v1/lmss_controller_spec.rb b/spec/controllers/api/v1/lmss_controller_spec.rb index 19c6c27..c6118f7 100644 --- a/spec/controllers/api/v1/lmss_controller_spec.rb +++ b/spec/controllers/api/v1/lmss_controller_spec.rb @@ -2,26 +2,70 @@ module Api module V1 describe LmssController do - let(:mock_course_id) { 16 } - let(:mock_course_name) { 'testCourseName' } - describe 'create' do - it 'throws a 501 error' do - post :create, params: { - course_id: :mock_course_id, - name: :mock_course_name, - } - expect(response.status).to eq(501) + def json_response + JSON.parse(response.body) + end + + before do + # Manually create a course and LMS in the database + @course = Course.create!(course_name: "Mock CS169 Course") + @lms = Lms.create!(lms_name: "Mock Canvas", use_auth_token: true) + end + + after do + # Clean up the specifically created data + CourseToLms.delete_all + Course.delete_all + Lms.delete_all + end + + describe 'POST #create' do + context 'when valid parameters are provided' do + it 'creates a new course_to_lms association and returns status :created' do + post :create, params: { course_id: @course.id, lms_id: @lms.id } + expect(response).to have_http_status(:created) + expect(json_response['course_id']).to eq(@course.id) + expect(json_response['lms_id']).to eq(@lms.id) + end end - it 'throws a 401 error if the name is not specified' do - post :create, params: { - course_id: :mock_course_id, - } - expect(response.status).to eq(401) - expect(response.body).to eq('name parameter is required') + + context 'when lms_id is missing' do + it 'returns status :bad_request' do + post :create, params: { course_id: @course.id } + expect(response).to have_http_status(:bad_request) + expect(response.body).to include('course_id and lms_id are required') + end + end + + context 'when course does not exist' do + it 'returns status :not_found' do + post :create, params: { course_id: -1, lms_id: @lms.id } + expect(response).to have_http_status(:not_found) + expect(response.body).to include('Course not found') + end + end + + context 'when lms does not exist' do + it 'returns status :not_found' do + post :create, params: { course_id: @course.id, lms_id: -1 } + expect(response).to have_http_status(:not_found) + expect(response.body).to include('Lms not found') + end + end + + context 'when the association already exists' do + it 'returns status :ok' do + CourseToLms.create!(course_id: @course.id, lms_id: @lms.id) + post :create, params: { course_id: @course.id, lms_id: @lms.id } + expect(response).to have_http_status(:ok) + expect(response.body).to include('The association between the specified course and LMS already exists.') + end end end + + describe 'index' do it 'throws a 501 error' do get :index, params: { course_id: :mock_course_id } From 03264c747b5b6352c4788cbf005797728c768d6b Mon Sep 17 00:00:00 2001 From: Evan Kandell Date: Fri, 19 Apr 2024 18:22:32 -0700 Subject: [PATCH 04/12] added space in schema for canvas data --- .../20240420012042_add_external_course_id_to_courses.rb | 5 +++++ ...0240420012054_add_external_user_id_to_lms_credentials.rb | 5 +++++ ...40420012105_add_external_assignment_id_to_assignments.rb | 5 +++++ ...0240420012126_add_external_extension_id_to_extensions.rb | 5 +++++ db/schema.rb | 6 +++++- 5 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20240420012042_add_external_course_id_to_courses.rb create mode 100644 db/migrate/20240420012054_add_external_user_id_to_lms_credentials.rb create mode 100644 db/migrate/20240420012105_add_external_assignment_id_to_assignments.rb create mode 100644 db/migrate/20240420012126_add_external_extension_id_to_extensions.rb diff --git a/db/migrate/20240420012042_add_external_course_id_to_courses.rb b/db/migrate/20240420012042_add_external_course_id_to_courses.rb new file mode 100644 index 0000000..a05e460 --- /dev/null +++ b/db/migrate/20240420012042_add_external_course_id_to_courses.rb @@ -0,0 +1,5 @@ +class AddExternalCourseIdToCourses < ActiveRecord::Migration[7.1] + def change + add_column :courses, :external_course_id, :string + end +end diff --git a/db/migrate/20240420012054_add_external_user_id_to_lms_credentials.rb b/db/migrate/20240420012054_add_external_user_id_to_lms_credentials.rb new file mode 100644 index 0000000..4d8b932 --- /dev/null +++ b/db/migrate/20240420012054_add_external_user_id_to_lms_credentials.rb @@ -0,0 +1,5 @@ +class AddExternalUserIdToLmsCredentials < ActiveRecord::Migration[7.1] + def change + add_column :lms_credentials, :external_user_id, :string + end +end diff --git a/db/migrate/20240420012105_add_external_assignment_id_to_assignments.rb b/db/migrate/20240420012105_add_external_assignment_id_to_assignments.rb new file mode 100644 index 0000000..a05f74c --- /dev/null +++ b/db/migrate/20240420012105_add_external_assignment_id_to_assignments.rb @@ -0,0 +1,5 @@ +class AddExternalAssignmentIdToAssignments < ActiveRecord::Migration[7.1] + def change + add_column :assignments, :external_assignment_id, :string + end +end diff --git a/db/migrate/20240420012126_add_external_extension_id_to_extensions.rb b/db/migrate/20240420012126_add_external_extension_id_to_extensions.rb new file mode 100644 index 0000000..9a8cb56 --- /dev/null +++ b/db/migrate/20240420012126_add_external_extension_id_to_extensions.rb @@ -0,0 +1,5 @@ +class AddExternalExtensionIdToExtensions < ActiveRecord::Migration[7.1] + def change + add_column :extensions, :external_extension_id, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index cd56123..9c4cc9f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_03_14_230145) do +ActiveRecord::Schema[7.1].define(version: 2024_04_20_012126) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -19,6 +19,7 @@ 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" end @@ -35,6 +36,7 @@ t.string "course_name" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "external_course_id" end create_table "extensions", force: :cascade do |t| @@ -45,6 +47,7 @@ t.bigint "last_processed_by_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "external_extension_id" t.index ["assignment_id"], name: "index_extensions_on_assignment_id" t.index ["last_processed_by_id"], name: "index_extensions_on_last_processed_by_id" end @@ -58,6 +61,7 @@ t.string "refresh_token" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "external_user_id" t.index ["user_id"], name: "index_lms_credentials_on_user_id" end From 99ff5e0b9696dfb110e96eee0bd2c37a721bce91 Mon Sep 17 00:00:00 2001 From: Zzz212zzZ <2120020201@qq.com> Date: Sat, 20 Apr 2024 14:32:44 -0700 Subject: [PATCH 05/12] Replace the external_course_id from courses table to course_to_lmss table --- ...20240420211050_add_external_course_id_to_courseto_lmss.rb | 5 +++++ .../20240420211809_remove_external_course_id_from_courses.rb | 5 +++++ db/schema.rb | 4 ++-- 3 files changed, 12 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20240420211050_add_external_course_id_to_courseto_lmss.rb create mode 100644 db/migrate/20240420211809_remove_external_course_id_from_courses.rb diff --git a/db/migrate/20240420211050_add_external_course_id_to_courseto_lmss.rb b/db/migrate/20240420211050_add_external_course_id_to_courseto_lmss.rb new file mode 100644 index 0000000..604533d --- /dev/null +++ b/db/migrate/20240420211050_add_external_course_id_to_courseto_lmss.rb @@ -0,0 +1,5 @@ +class AddExternalCourseIdToCoursetoLmss < ActiveRecord::Migration[7.1] + def change + add_column :course_to_lmss, :external_course_id, :string + end +end \ No newline at end of file diff --git a/db/migrate/20240420211809_remove_external_course_id_from_courses.rb b/db/migrate/20240420211809_remove_external_course_id_from_courses.rb new file mode 100644 index 0000000..c74ceca --- /dev/null +++ b/db/migrate/20240420211809_remove_external_course_id_from_courses.rb @@ -0,0 +1,5 @@ +class RemoveExternalCourseIdFromCourses < ActiveRecord::Migration[7.1] + def change + remove_column :courses, :external_course_id, :string + end +end \ No newline at end of file diff --git a/db/schema.rb b/db/schema.rb index 9c4cc9f..186595a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_04_20_012126) do +ActiveRecord::Schema[7.1].define(version: 2024_04_20_211809) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -28,6 +28,7 @@ t.bigint "course_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "external_course_id" t.index ["course_id"], name: "index_course_to_lmss_on_course_id" t.index ["lms_id"], name: "index_course_to_lmss_on_lms_id" end @@ -36,7 +37,6 @@ t.string "course_name" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.string "external_course_id" end create_table "extensions", force: :cascade do |t| From 6b6f81f9d0a3a970d1a98192031dd89a3c57ed3d Mon Sep 17 00:00:00 2001 From: Zzz212zzZ <2120020201@qq.com> Date: Sat, 20 Apr 2024 15:05:33 -0700 Subject: [PATCH 06/12] Take external_course_id into consideration --- app/controllers/api/v1/lmss_controller.rb | 12 +++++++----- spec/controllers/api/v1/lmss_controller_spec.rb | 13 +++++++------ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/app/controllers/api/v1/lmss_controller.rb b/app/controllers/api/v1/lmss_controller.rb index cb15555..234183e 100644 --- a/app/controllers/api/v1/lmss_controller.rb +++ b/app/controllers/api/v1/lmss_controller.rb @@ -5,8 +5,9 @@ class LmssController < BaseController # POST /courses/:course_id/lmss def create - course_id = params[:course_id] - lms_id = params[:lms_id] + course_id = params[:course_id] # course_id from the URL + lms_id = params[:lms_id] # lms_id from the body + external_course_id = params[:external_course_id] # external_course_id from the body # Ensure the course exists unless Course.exists?(course_id) @@ -21,7 +22,7 @@ def create end # Check if the entry already exists - existing_entry = CourseToLms.find_by(course_id: course_id, lms_id: lms_id) + existing_entry = CourseToLms.find_by(course_id: course_id, lms_id: lms_id, external_course_id: external_course_id) if existing_entry render json: { message: 'The association between the specified course and LMS already exists.' }, status: :ok return @@ -30,7 +31,8 @@ def create # Create the course_to_lms entry course_to_lms = CourseToLms.new( course_id: course_id, - lms_id: lms_id + lms_id: lms_id, + external_course_id: external_course_id ) if course_to_lms.save @@ -54,7 +56,7 @@ def destroy # TODO: this validation should also check the config file for the name of lms's. # def validate_ids! - if params[:course_id].blank? || params[:lms_id].blank? + if params[:course_id].blank? || params[:lms_id].blank? || params[:external_course_id].blank? render json: { error: 'course_id and lms_id are required' }, status: :bad_request end end diff --git a/spec/controllers/api/v1/lmss_controller_spec.rb b/spec/controllers/api/v1/lmss_controller_spec.rb index c6118f7..68d64ca 100644 --- a/spec/controllers/api/v1/lmss_controller_spec.rb +++ b/spec/controllers/api/v1/lmss_controller_spec.rb @@ -10,6 +10,7 @@ def json_response # Manually create a course and LMS in the database @course = Course.create!(course_name: "Mock CS169 Course") @lms = Lms.create!(lms_name: "Mock Canvas", use_auth_token: true) + @external_course_id = "mock_external_course_id" end after do @@ -22,7 +23,7 @@ def json_response describe 'POST #create' do context 'when valid parameters are provided' do it 'creates a new course_to_lms association and returns status :created' do - post :create, params: { course_id: @course.id, lms_id: @lms.id } + post :create, params: { course_id: @course.id, lms_id: @lms.id, external_course_id: @external_course_id} expect(response).to have_http_status(:created) expect(json_response['course_id']).to eq(@course.id) expect(json_response['lms_id']).to eq(@lms.id) @@ -32,7 +33,7 @@ def json_response context 'when lms_id is missing' do it 'returns status :bad_request' do - post :create, params: { course_id: @course.id } + post :create, params: { course_id: @course.id, external_course_id: @external_course_id} expect(response).to have_http_status(:bad_request) expect(response.body).to include('course_id and lms_id are required') end @@ -40,7 +41,7 @@ def json_response context 'when course does not exist' do it 'returns status :not_found' do - post :create, params: { course_id: -1, lms_id: @lms.id } + post :create, params: { course_id: -1, lms_id: @lms.id, external_course_id: @external_course_id} expect(response).to have_http_status(:not_found) expect(response.body).to include('Course not found') end @@ -48,7 +49,7 @@ def json_response context 'when lms does not exist' do it 'returns status :not_found' do - post :create, params: { course_id: @course.id, lms_id: -1 } + post :create, params: { course_id: @course.id, lms_id: -1, external_course_id: @external_course_id} expect(response).to have_http_status(:not_found) expect(response.body).to include('Lms not found') end @@ -56,8 +57,8 @@ def json_response context 'when the association already exists' do it 'returns status :ok' do - CourseToLms.create!(course_id: @course.id, lms_id: @lms.id) - post :create, params: { course_id: @course.id, lms_id: @lms.id } + CourseToLms.create!(course_id: @course.id, lms_id: @lms.id, external_course_id: @external_course_id) + post :create, params: { course_id: @course.id, lms_id: @lms.id, external_course_id: @external_course_id} expect(response).to have_http_status(:ok) expect(response.body).to include('The association between the specified course and LMS already exists.') end From 68b45e14a15a42c449a9fefed46f9e1eabac7327 Mon Sep 17 00:00:00 2001 From: Zzz212zzZ <2120020201@qq.com> Date: Sat, 20 Apr 2024 17:17:55 -0700 Subject: [PATCH 07/12] Adjust Assignment Table, which should rely on course_to_lms instead of only lms --- ...8_change_lms_id_to_course_to_lms_id_in_assignments.rb | 9 +++++++++ db/schema.rb | 8 ++++---- 2 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20240420232708_change_lms_id_to_course_to_lms_id_in_assignments.rb diff --git a/db/migrate/20240420232708_change_lms_id_to_course_to_lms_id_in_assignments.rb b/db/migrate/20240420232708_change_lms_id_to_course_to_lms_id_in_assignments.rb new file mode 100644 index 0000000..c3a670f --- /dev/null +++ b/db/migrate/20240420232708_change_lms_id_to_course_to_lms_id_in_assignments.rb @@ -0,0 +1,9 @@ +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_index :assignments, :course_to_lms_id + add_foreign_key :assignments, :course_to_lmss, column: :course_to_lms_id + end +end diff --git a/db/schema.rb b/db/schema.rb index 186595a..754221d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,17 +10,17 @@ # # 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 + t.index ["course_to_lms_id"], name: "index_assignments_on_course_to_lms_id" end create_table "course_to_lmss", force: :cascade do |t| @@ -89,7 +89,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" From 4cb45928cdd0f6dea53afd34bad7035ee0d72790 Mon Sep 17 00:00:00 2001 From: Zzz212zzZ <2120020201@qq.com> Date: Sat, 20 Apr 2024 17:18:23 -0700 Subject: [PATCH 08/12] Add assignment model --- app/models/assignment.rb | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 app/models/assignment.rb diff --git a/app/models/assignment.rb b/app/models/assignment.rb new file mode 100644 index 0000000..45d1155 --- /dev/null +++ b/app/models/assignment.rb @@ -0,0 +1,7 @@ +# app/models/assignment.rb +class Assignment < ApplicationRecord + belongs_to :course_to_lms + + validates :name, presence: true + validates :external_assignment_id, presence: true + end \ No newline at end of file From f7a23336987a53eef158b29094c6196e60dfedf2 Mon Sep 17 00:00:00 2001 From: Zzz212zzZ <2120020201@qq.com> Date: Sat, 20 Apr 2024 17:19:00 -0700 Subject: [PATCH 09/12] Implement route of create in assignment controller --- .../api/v1/assignments_controller.rb | 55 ++++++++++++++++--- 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/app/controllers/api/v1/assignments_controller.rb b/app/controllers/api/v1/assignments_controller.rb index a43c0d9..07f9bed 100644 --- a/app/controllers/api/v1/assignments_controller.rb +++ b/app/controllers/api/v1/assignments_controller.rb @@ -1,20 +1,59 @@ module Api - module V1 - class AssignmentsController < ApplicationController + module V1 + class AssignmentsController < ApplicationController + before_action :validate_ids!, only: [:create] - def index - render json: 'not yet implemented', status: 501 + def index + render json: 'not yet implemented', status: 501 + end + + # POST /courses/:course_id/lms/:lms_id/assignments + def create + assignment_name = params[:name] + external_assignment_id = params[:external_assignment_id] + course_id = params[:course_id] + lms_id = params[:lms_id] + + # Retrieve the course_to_lms entry + course_to_lms = CourseToLms.find_by(course_id: course_id, lms_id: lms_id) + + unless course_to_lms + render json: { error: 'Course to LMS association not found' }, status: :not_found + return end - def create - render json: 'not yet implemented', status: 501 + # Check this assignment doesn't already exist + existing_assignment = Assignment.find_by(course_to_lms_id: course_to_lms.id, name: assignment_name, external_assignment_id: external_assignment_id) + if existing_assignment + render json: { message: 'The assignment with the specified external ID already exists.' }, status: :ok + return end - def destroy - render json: 'not yet implemented', status: 501 + # Create the assignment + assignment = Assignment.new( + course_to_lms_id: course_to_lms.id, + name: assignment_name, + external_assignment_id: 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: 'not yet implemented', status: 501 + end + + def validate_ids! + if params[:name].blank? || params[:external_assignment_id].blank? || params[:course_id].blank? || params[:lms_id].blank? + render json: { error: 'Course ID, LMS ID, name, and external assignment ID are required' }, status: :bad_request + end + end end end +end \ No newline at end of file From 694494f0934e58d04ad56354da4d5a70b1349381 Mon Sep 17 00:00:00 2001 From: Zzz212zzZ <2120020201@qq.com> Date: Sat, 20 Apr 2024 17:19:38 -0700 Subject: [PATCH 10/12] Add test for create in assignment controller --- .../api/v1/assignments_controller_spec.rb | 60 ++++++++++++++++--- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/spec/controllers/api/v1/assignments_controller_spec.rb b/spec/controllers/api/v1/assignments_controller_spec.rb index c50949f..621c804 100644 --- a/spec/controllers/api/v1/assignments_controller_spec.rb +++ b/spec/controllers/api/v1/assignments_controller_spec.rb @@ -1,28 +1,72 @@ require 'rails_helper' module Api module V1 - RSpec.describe AssignmentsController, type: :request do - let(:mock_course_id) { 1 } - let(:mock_lms_id) { 1 } + RSpec.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 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 + 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('Course to LMS association not found') + 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('The assignment with the specified external ID 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 From 569dc2e79c00fb3a594bee60d997e1bbe0e23c0a Mon Sep 17 00:00:00 2001 From: Zzz212zzZ <2120020201@qq.com> Date: Sat, 20 Apr 2024 17:44:33 -0700 Subject: [PATCH 11/12] Refactor to pass the Codeclimate --- .../api/v1/assignments_controller.rb | 50 +++++++++---------- .../api/v1/assignments_controller_spec.rb | 6 +-- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/app/controllers/api/v1/assignments_controller.rb b/app/controllers/api/v1/assignments_controller.rb index 07f9bed..3245b9b 100644 --- a/app/controllers/api/v1/assignments_controller.rb +++ b/app/controllers/api/v1/assignments_controller.rb @@ -9,51 +9,51 @@ def index # POST /courses/:course_id/lms/:lms_id/assignments def create - assignment_name = params[:name] - external_assignment_id = params[:external_assignment_id] - course_id = params[:course_id] - lms_id = params[:lms_id] + course_to_lms = fetch_course_to_lms(params[:course_id], params[:lms_id]) + return unless course_to_lms - # Retrieve the course_to_lms entry - course_to_lms = CourseToLms.find_by(course_id: course_id, lms_id: lms_id) + return if assignment_exists?(course_to_lms, params[:name], params[:external_assignment_id]) + + create_and_render_assignment(course_to_lms, params[:name], params[:external_assignment_id]) + end + def destroy + render json: 'not yet implemented', status: 501 + end + + private + + def fetch_course_to_lms(course_id, lms_id) + course_to_lms = CourseToLms.find_by(course_id: course_id, lms_id: lms_id) unless course_to_lms - render json: { error: 'Course to LMS association not found' }, status: :not_found - return + render json: { error: 'No such Course_LMS association' }, status: :not_found end + course_to_lms + end - # Check this assignment doesn't already exist + def assignment_exists?(course_to_lms, assignment_name, external_assignment_id) existing_assignment = Assignment.find_by(course_to_lms_id: course_to_lms.id, name: assignment_name, external_assignment_id: external_assignment_id) if existing_assignment - render json: { message: 'The assignment with the specified external ID already exists.' }, status: :ok - return + render json: { message: 'Record already exists' }, status: :ok + return true end + false + end - # Create the assignment - assignment = Assignment.new( - course_to_lms_id: course_to_lms.id, - name: assignment_name, - external_assignment_id: external_assignment_id - ) - + def create_and_render_assignment(course_to_lms, assignment_name, external_assignment_id) + assignment = Assignment.new(course_to_lms_id: course_to_lms.id, name: assignment_name, external_assignment_id: 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: 'not yet implemented', status: 501 end def validate_ids! if params[:name].blank? || params[:external_assignment_id].blank? || params[:course_id].blank? || params[:lms_id].blank? - render json: { error: 'Course ID, LMS ID, name, and external assignment ID are required' }, status: :bad_request + render json: { error: 'Params required' }, status: :bad_request end end end end end - \ No newline at end of file diff --git a/spec/controllers/api/v1/assignments_controller_spec.rb b/spec/controllers/api/v1/assignments_controller_spec.rb index 621c804..27f5245 100644 --- a/spec/controllers/api/v1/assignments_controller_spec.rb +++ b/spec/controllers/api/v1/assignments_controller_spec.rb @@ -11,7 +11,7 @@ def json_response 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 @@ -46,7 +46,7 @@ def json_response it 'returns status :not_found' do 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('Course to LMS association not found') + expect(response.body).to include('No such Course_LMS association') end end @@ -55,7 +55,7 @@ def json_response 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('The assignment with the specified external ID already exists.') + expect(response.body).to include('Record already exists') end end From c66a0604228325db0f076c084acbe423b730aad7 Mon Sep 17 00:00:00 2001 From: Zzz212zzZ <2120020201@qq.com> Date: Mon, 22 Apr 2024 11:28:21 -0700 Subject: [PATCH 12/12] Refactor assignment controller and check on the codeclimate again --- app/controllers/api/v1/assignments_controller.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/v1/assignments_controller.rb b/app/controllers/api/v1/assignments_controller.rb index 3245b9b..769cfd7 100644 --- a/app/controllers/api/v1/assignments_controller.rb +++ b/app/controllers/api/v1/assignments_controller.rb @@ -9,11 +9,12 @@ def index # POST /courses/:course_id/lms/:lms_id/assignments def create + # Check if the course_to_lms association exists course_to_lms = fetch_course_to_lms(params[:course_id], params[:lms_id]) return unless course_to_lms - + # Check if the assignment already exists return if assignment_exists?(course_to_lms, params[:name], params[:external_assignment_id]) - + # Create and render the assignment create_and_render_assignment(course_to_lms, params[:name], params[:external_assignment_id]) end