From 601a5c7328fce0d17f0a08f88a1ad563a0f4f357 Mon Sep 17 00:00:00 2001 From: Evan Kandell <86220777+ekandell@users.noreply.github.com> Date: Mon, 29 Apr 2024 17:25:56 -0700 Subject: [PATCH] Final workflow (#47) * modified extensions_controller for extension setup flow * fixed extensions_controller_spec.rb * updated extensions_controller_spec.rb as per extensions_controller * fixed routes for extensions_controller_spec.rb * Add models for course_to_lms * Implement creating association in course_to_lms * Test for creating association in course_to_lms * correcting rspec for extensions controller * testing of api call completed * quick typo fix * added space in schema for canvas data * Replace the external_course_id from courses table to course_to_lmss table * More detailed extensions req, still needs more * Take external_course_id into consideration * I think set up correctly, needs rest of models to test * 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 * test coverage badge update * test coverage badge update 2 * code climate yml file updated * code climate yml file updated 2 * badge fix to linux * added codeclimate test reporter * added gems * simple cov version downgraded * simplecov v17 * secret added * secret added2 * secret added3 * secret added4 * initial changes to create user * drafted * validation email added * chnages added: new+save, has many asso for extension * Merge latest main into feature branch and resolve conflict * Refactor the controller and add integer validation * tests pass. finally. * improving test coverage * slightly clarified tests * remove render json of strings * Fix potential issue in lmss_controller * Fix minor issue based on pr comments * Add one test to reach a full LOC test coverage * starting to refactor * further refactoring * fixing everything that broke because someone decided to change the schema without telling anyone else... * everything works for demo * fixing failing tests due to prev updates * missed one merge * including validation helper * fixed comments as per PR46 (except line 30 of extensions_controller_spec, idk why it errors when I try to update those hashes to new syntax) * fix: ensure test succeed * style: format for readability --------- Co-authored-by: zee6197 Co-authored-by: Zzz212zzZ <2120020201@qq.com> Co-authored-by: Sepehr Behmanesh Fard Co-authored-by: Connor Bernard --- .gitignore | 2 + Gemfile | 1 + Gemfile.lock | 10 ++ .../api/v1/assignments_controller.rb | 4 +- .../api/v1/extensions_controller.rb | 69 ++++++- app/facades/canvas_facade.rb | 13 +- app/models/extension.rb | 9 +- config/environments/development.rb | 2 + db/seeds.rb | 48 ++--- spec/Facades/canvas_facade_spec.rb | 4 +- .../api/v1/assignments_controller_spec.rb | 12 +- .../api/v1/courses_controller_spec.rb | 12 +- .../api/v1/extensions_controller_spec.rb | 169 ++++++++++++++---- .../api/v1/lmss_controller_spec.rb | 11 +- spec/spec_helper.rb | 1 + 15 files changed, 260 insertions(+), 107 deletions(-) diff --git a/.gitignore b/.gitignore index 1cb38ca..2910ee8 100644 --- a/.gitignore +++ b/.gitignore @@ -71,3 +71,5 @@ yarn-debug.log* /config/master.key /config/credentials/production.key + +.DS_Store \ No newline at end of file diff --git a/Gemfile b/Gemfile index 70df564..63eca33 100644 --- a/Gemfile +++ b/Gemfile @@ -79,6 +79,7 @@ group :test do gem 'cucumber-rails-training-wheels' gem 'database_cleaner' gem 'timecop' + gem 'webmock' end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index 6599590..7aa4620 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -103,6 +103,9 @@ GEM coderay (1.1.3) concurrent-ruby (1.2.3) connection_pool (2.4.1) + crack (1.0.0) + bigdecimal + rexml crass (1.0.6) cucumber (9.1.2) builder (~> 3.2, >= 3.2.4) @@ -176,6 +179,7 @@ GEM guard (~> 2.1) guard-compat (~> 1.1) rspec (>= 2.99.0, < 4.0) + hashdiff (1.1.0) httparty (0.21.0) mini_mime (>= 1.0.0) multi_xml (>= 0.5.2) @@ -310,6 +314,7 @@ GEM regexp_parser (2.9.0) reline (0.4.2) io-console (~> 0.5) + rexml (3.2.6) rspec (3.13.0) rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) @@ -376,6 +381,10 @@ GEM activemodel (>= 6.0.0) bindex (>= 0.4.0) railties (>= 6.0.0) + webmock (3.23.0) + addressable (>= 2.8.0) + crack (>= 0.3.2) + hashdiff (>= 0.4.0, < 2.0.0) webrick (1.8.1) websocket-driver (0.7.6) websocket-extensions (>= 0.1.0) @@ -423,6 +432,7 @@ DEPENDENCIES turbo-rails tzinfo-data web-console + webmock RUBY VERSION ruby 3.3.0p0 diff --git a/app/controllers/api/v1/assignments_controller.rb b/app/controllers/api/v1/assignments_controller.rb index 9ab4fe6..ef6881f 100644 --- a/app/controllers/api/v1/assignments_controller.rb +++ b/app/controllers/api/v1/assignments_controller.rb @@ -2,7 +2,9 @@ module Api module V1 class AssignmentsController < ApplicationController include CanvasValidationHelper + before_action :validate_ids!, only: [:create] + skip_before_action :verify_authenticity_token def index render json: { message: 'not yet implemented'} , status: 501 @@ -12,7 +14,6 @@ def index 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 @@ -23,7 +24,6 @@ def create render json: { message: 'Record already exists' }, status: :ok return end - # 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 diff --git a/app/controllers/api/v1/extensions_controller.rb b/app/controllers/api/v1/extensions_controller.rb index 414e211..b9f1b74 100644 --- a/app/controllers/api/v1/extensions_controller.rb +++ b/app/controllers/api/v1/extensions_controller.rb @@ -1,18 +1,71 @@ module Api module V1 class ExtensionsController < BaseController - def create - render :json => 'not yet implemented'.to_json, status: 501 - end - + before_action :set_facade + def index - render :json => 'not yet implemented'.to_json, status: 501 + render json: { message: 'not yet implemented'}, status: 501 + end + + def create + find_extension_params + # Get External Assignment object to find initial due date + assignment_response = @canvas_facade.get_assignment( + @course_to_lms.external_course_id.to_i, + @assignment.external_assignment_id.to_i, + ) + if (assignment_response.status != 200) + render json: assignment_response.to_json, status: 500 + return + end + assignment_json = JSON.parse(assignment_response.body) + + # Provision Extension + response = @canvas_facade.provision_extension( + @course_to_lms.external_course_id.to_i, + params[:student_uid].to_i, + @assignment.external_assignment_id.to_i, + params[:new_due_date], + ) + if !response.success? + render json: response.body, status: response.status + return + end + assignment_override = JSON.parse(response.body) + + @extension = Extension.new( + assignment_id: @assignment.id, + student_email: nil, + initial_due_date: assignment_json["due_at"], + new_due_date: assignment_override["due_at"], + external_extension_id: assignment_override["id"], + last_processed_by_id: nil, + ) + if !@extension.save + render json: {"error": "Extension requested, but local save failed"}.to_json, status: 500 + return + end + render json: @extension.to_json, status: 200 end def destroy - render :json => 'not yet implemented'.to_json, status: 501 + render json: { message: 'not yet implemented'}, status: 501 + end + + private + + def set_facade + Rails.logger.info "Using CANVAS_URL: #{ENV['CANVAS_URL']}" + # not sure if auth key will be in the request headers or in cookie + @canvas_facade = CanvasFacade.new(request.headers['Authorization']) + end + + def find_extension_params + @lms = Lms.find(params[:lms_id]) + @course = Course.find(params[:course_id]) + @assignment = Assignment.find(params[:assignment_id]) + @course_to_lms = CourseToLms.find(@assignment.course_to_lms_id) end - end end - end \ No newline at end of file + end diff --git a/app/facades/canvas_facade.rb b/app/facades/canvas_facade.rb index 461ba02..976440e 100644 --- a/app/facades/canvas_facade.rb +++ b/app/facades/canvas_facade.rb @@ -82,11 +82,13 @@ def get_assignment_overrides(courseId, assignmentId) # @return [Faraday::Response] information about the new override. def create_assignment_override(courseId, assignmentId, studentIds, title, dueDate, unlockDate, lockDate) @canvasApi.post("courses/#{courseId}/assignments/#{assignmentId}/overrides", { - student_ids: studentIds, - title: title, - due_at: dueDate, - unlock_at: unlockDate, - lock_at: lockDate, + assignment_override: { + student_ids: studentIds, + title: title, + due_at: dueDate, + unlock_at: unlockDate, + lock_at: lockDate, + } }) end @@ -143,7 +145,6 @@ def provision_extension(courseId, studentId, assignmentId, newDueDate) get_current_formatted_time(), newDueDate, ) - # Either successful or error that is not explicitly handled here. if (createOverrideResponse.status != 400) return createOverrideResponse diff --git a/app/models/extension.rb b/app/models/extension.rb index ea5f41b..e4f6aa7 100644 --- a/app/models/extension.rb +++ b/app/models/extension.rb @@ -1,7 +1,14 @@ +# assignment_id: foreign key to local assignment +# student_email: requires another api request to find student data (sid is given in first response). This currently doesn't exist in CanvasFacade +# initial_due_date: also requires an api request to find assignment data (assignment id is given in first response) +# Note that the assignment.due_at shows the due date as it is for whoever's logged in (which if it's a teacher, should be the original due date) but the actual original due date is never saved anywhere +# new_due_date: +# external_extension_id: +# last_processed_by_id: Requires login/sessions to be properly implemented class Extension < ApplicationRecord #Relationship with Assignment belongs_to :assignment #Relationship with User has_one :user -end \ No newline at end of file +end diff --git a/config/environments/development.rb b/config/environments/development.rb index 8f64d3a..b8ac1b7 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -77,4 +77,6 @@ # Raise error when a before_action's only/except options reference missing actions config.action_controller.raise_on_missing_callback_actions = true + + config.hosts << "flextensions.lvh.me:3000" end diff --git a/db/seeds.rb b/db/seeds.rb index dc36945..4cc4299 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -19,14 +19,7 @@ canvas = Lms.create!({ lms_name: "Canvas", - use_auth_token: true -}) - - -test_assignment = Assignment.create!({ - lms_id: canvas.id, - name: "Test Assignment", - external_assignment_id: "11111" + use_auth_token: true, }) @@ -37,7 +30,13 @@ test_course_to_lms = CourseToLms.create!({ lms_id: canvas.id, course_id: test_course.id, - external_course_id: "22222" + external_course_id: "22222", +}) + +test_assignment = Assignment.create!({ + course_to_lms_id: test_course_to_lms.id, + name: "Test Assignment", + external_assignment_id: "11111", }) test_user = User.create!({ @@ -47,7 +46,7 @@ test_user_to_course = UserToCourse.create!({ user_id: test_user.id, course_id: test_course.id, - role: "test" + role: "test", }) test_extension = Extension.create!({ @@ -56,37 +55,12 @@ initial_due_date: DateTime.iso8601('2024-04-20'), new_due_date: DateTime.iso8601('2024-04-30'), last_processed_by_id: test_user.id, - external_extension_id: "33333" + external_extension_id: "33333", }) test_lms_credential = LmsCredential.create!({ user_id: test_user.id, lms_name: "canvas", token: "test token", - external_user_id: "44444" -}) - -real_course = Course.create!({ - course_name: "CS169L Flextension", -}) - -real_course_to_lms = CourseToLms.create!({ - lms_id: canvas.id, - course_id: real_course.id, - external_course_id: "1534405" + external_user_id: "44444", }) - -real_assignment = Assignment.create!({ - lms_id: canvas.id, - name: "Seed Data Testing", - external_assignment_id: "8741483" -}) - -real_extension = Extension.create!({ - assignment_id: real_assignment.id, - student_email: "teststudent@example.com", - initial_due_date: DateTime.iso8601('2024-04-20'), - new_due_date: DateTime.iso8601('2024-04-27'), - last_processed_by_id: test_user.id, - external_extension_id: "270163" -}) \ No newline at end of file diff --git a/spec/Facades/canvas_facade_spec.rb b/spec/Facades/canvas_facade_spec.rb index 82aa1a2..91d1267 100644 --- a/spec/Facades/canvas_facade_spec.rb +++ b/spec/Facades/canvas_facade_spec.rb @@ -119,13 +119,13 @@ it 'has correct request body' do expect(conn).to receive(:post).with( createAssignmentOverrideUrl, - { + { assignment_override: { student_ids: [mockStudentId], title: mockTitle, due_at: mockDate, unlock_at: mockDate, lock_at: mockDate, - } + }} ) facade.create_assignment_override( diff --git a/spec/controllers/api/v1/assignments_controller_spec.rb b/spec/controllers/api/v1/assignments_controller_spec.rb index c69f012..fef0236 100644 --- a/spec/controllers/api/v1/assignments_controller_spec.rb +++ b/spec/controllers/api/v1/assignments_controller_spec.rb @@ -19,10 +19,14 @@ def json_response end after do - Assignment.delete_all - CourseToLms.delete_all - Course.delete_all - Lms.delete_all + LmsCredential.destroy_all + Extension.destroy_all + Assignment.destroy_all + CourseToLms.destroy_all + UserToCourse.destroy_all + Course.destroy_all + Lms.destroy_all + User.destroy_all end describe "GET /api/v1/courses/:course_id/lmss/:lms_id/assignments" do diff --git a/spec/controllers/api/v1/courses_controller_spec.rb b/spec/controllers/api/v1/courses_controller_spec.rb index ad9b297..6846b8b 100644 --- a/spec/controllers/api/v1/courses_controller_spec.rb +++ b/spec/controllers/api/v1/courses_controller_spec.rb @@ -5,23 +5,23 @@ module V1 describe 'POST #create' do context "when the new course is successfully created" do let(:course_name) { "New Course" } - + it "creates and saves a new course" do post :create, params: { course_name: course_name } - + expect(response).to have_http_status(:created) expect(Course.find_by(course_name: course_name)).to be_present expect(flash[:success]).to eq("Course created successfully") expect(JSON.parse(response.body)['course_name']).to eq('New Course') end end - + context "when a course with the same name already exists" do let!(:existing_course) { Course.create(course_name: "Existing Course") } - + it "does not create a new course with the same name and returns an error" do post :create, params: { course_name: existing_course.course_name } - + expect(Course.find_by(course_name: existing_course.course_name)).to be_present expect(response).to have_http_status(:unprocessable_entity) expect(JSON.parse(response.body)).to eq({ "message" => "A course with the same course name already exists." }) @@ -45,7 +45,7 @@ module V1 describe 'add_user' do let(:test_course) { Course.create(course_name: "Test Course") } - let(:test_user) { User.create(email: "testuser@example.com") } + let(:test_user) { User.create!(email: "testuniqueuser@example.com") } context "Provided parameters are valid" do it "adds an existing user to an existing course" do diff --git a/spec/controllers/api/v1/extensions_controller_spec.rb b/spec/controllers/api/v1/extensions_controller_spec.rb index 8098498..ac54bc6 100644 --- a/spec/controllers/api/v1/extensions_controller_spec.rb +++ b/spec/controllers/api/v1/extensions_controller_spec.rb @@ -1,46 +1,139 @@ require 'rails_helper' +require 'byebug' module Api module V1 describe ExtensionsController do - let(:mock_course_id) { 16 } - let(:mock_lms_id) { 1 } - let(:mock_assignment_id) { 9 } - let(:mock_extension_id) {5} - - - - ### TODO: check that it's properly handling post body as well as params. - describe 'create' do - it 'throws a 501 error' do - post :create, params: { - course_id: :mock_course_id, - lms_id: :mock_lms_id, - assignment_id: :mock_assignment_id - } - expect(response.status).to eq(501) + describe "POST /api/v1/courses/:course_id/lmss/:lms_id/assignments/:assignment_id/extensions" do + before(:all) do + load "#{Rails.root}/db/seeds.rb" + @course = Course.take + @assignment = Assignment.take + @extension = Extension.take + @lms = Lms.take + @course_to_lms = CourseToLms.find(@assignment.course_to_lms_id) + @mock_student_uid = 123 + @mock_new_due_date = '2024-04-16T16:00:00Z' + @auth_token = 'some_valid_token' + @mock_assignments_url = "#{ENV['CANVAS_URL']}/api/v1/courses/#{@course_to_lms.external_course_id}/assignments" + @mock_override_url = "#{@mock_assignments_url}/#{@assignment.external_assignment_id}/overrides" + end + + context "with valid parameters" do + it "creates a new extension and returns a success status" do + stub_request(:post, @mock_override_url). + with( + body: { + assignment_override: hash_including({ + "due_at" => @mock_new_due_date, + "lock_at" => @mock_new_due_date, + "student_ids" => [@mock_student_uid.to_s], + "title" => "#{@mock_student_uid} extended to #{@mock_new_due_date}", + }) + }, + headers: { Authorization: "Bearer #{@auth_token}" } + ).to_return( + status: 200, + body: { due_at: @mock_new_due_date, id: "3333" }.to_json, + headers: {}, + ) + stub_request(:get, "#{@mock_assignments_url}/#{@assignment.external_assignment_id}") + .to_return( + status: 200, + body: { due_at: '2024-04-13T16:00:00Z' }.to_json, + headers: {}, + ) + + request.headers.merge!({'Authorization': @auth_token}) + post :create, params: { + course_id: @course.id, + lms_id: @lms.id, + assignment_id: @assignment.id, + student_uid: @mock_student_uid, + new_due_date: @mock_new_due_date, + } + expect(response).to have_http_status(:success) + end + end + + context "with missing parameters" do + it "raises an error" do + stub_request(:post, @mock_override_url). + to_return(status: 400) + + stub_request(:get, "#{@mock_assignments_url}/#{@assignment.external_assignment_id}") + .to_return( + status: 200, + body: { due_at: '2024-04-13T16:00:00Z' }.to_json, + headers: {}, + ) + + expect { post(:create, params: { + course_id: @course.id, + lms_id: @lms.id, + assignment_id: @assignment.id, + }) }.to raise_error(FailedPipelineError) + end + end + + context "when canvas returns 500" do + it "returns a 500 status" do + stub_request(:post, @mock_override_url). + to_return( + status: 500, + body: { errors: ["unknown student ids"] }.to_json, + ) + + stub_request(:get, "#{@mock_assignments_url}/#{@assignment.external_assignment_id}") + .to_return( + status: 200, + body: { due_at: '2024-04-13T16:00:00Z' }.to_json, + headers: {}, + ) + + post :create, params: { + course_id: @course.id, + lms_id: @lms.id, + assignment_id: @assignment.id, + student_uid: 9999, + new_due_date: 7, + } + expect(response).to have_http_status(500) + end + end + + context "get assignment api call fails" do + it "doesn't request a new extension and returns a server error" do + stub_request(:post, @mock_override_url). + with( + body: hash_including({ + due_at: @mock_new_due_date, + lock_at: @mock_new_due_date, + student_ids: [@mock_student_uid.to_s], + title: "#{@mock_student_uid} extended to #{@mock_new_due_date}" + }), + headers: { Authorization: "Bearer #{@auth_token}" } + ). + to_return( + status: 200, + body: { due_at: @mock_new_due_date, id: "3333" }.to_json, + headers: {} + ) + + stub_request(:get, "#{@mock_assignments_url}/#{@assignment.external_assignment_id}") + .to_return(status: 404) + + request.headers.merge!({ Authorization: @auth_token }) + post :create, params: { + course_id: @course.id, + lms_id: @lms.id, + assignment_id: @assignment.id, + student_uid: @mock_student_uid, + new_due_date: @mock_new_due_date + } + expect(response).to have_http_status(:error) + end end - end - describe 'index' do - it 'throws a 501 error' do - post :index, params: { - course_id: :mock_course_id, - lms_id: :mock_lms_id, - assignment_id: :mock_assignment_id - } - expect(response.status).to eq(501) end - end - describe 'destroy' do - it 'throws a 501 error' do - delete :destroy, params: { - course_id: :mock_course_id, - lms_id: :mock_lms_id, - assignment_id: :mock_assignment_id, - id: :mock_extension_id - } - expect(response.status).to eq(501) - end -end end end -end \ No newline at end of file +end diff --git a/spec/controllers/api/v1/lmss_controller_spec.rb b/spec/controllers/api/v1/lmss_controller_spec.rb index 9da5084..d174920 100644 --- a/spec/controllers/api/v1/lmss_controller_spec.rb +++ b/spec/controllers/api/v1/lmss_controller_spec.rb @@ -15,9 +15,14 @@ def json_response after do # Clean up the specifically created data - CourseToLms.delete_all - Course.delete_all - Lms.delete_all + LmsCredential.destroy_all + Extension.destroy_all + Assignment.destroy_all + CourseToLms.destroy_all + UserToCourse.destroy_all + Course.destroy_all + Lms.destroy_all + User.destroy_all end describe 'POST #create' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 53d8c52..5864934 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -12,6 +12,7 @@ # the additional setup, and require it from the spec files that actually need # it. # +require 'webmock/rspec' # See https://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration require 'codeclimate-test-reporter'