diff --git a/app/controllers/uploaded_files_controller.rb b/app/controllers/uploaded_files_controller.rb index 63dde27a..4332bc19 100644 --- a/app/controllers/uploaded_files_controller.rb +++ b/app/controllers/uploaded_files_controller.rb @@ -1,5 +1,6 @@ class UploadedFilesController < ApplicationController before_action :authenticate_user! + before_action :validate_destroy_rights, only: [:destroy] # GET /uploaded_files def index @@ -12,6 +13,17 @@ def new @uploaded_file.author = current_user end + + # DELETE /uploaded_files/1 + def destroy + @uploaded_file.destroy + if @uploaded_file.allowsUpload.class.name == "Course" + redirect_to course_path(@uploaded_file.allowsUpload), notice: "File was successfully deleted." + else + redirect_to course_lecture_path(@uploaded_file.allowsUpload), notice: "File was successfully deleted." + end + end + # POST /uploaded_files def create file = uploaded_file_params["attachment"] @@ -48,4 +60,14 @@ def show def uploaded_file_params params.require(:uploaded_file).permit(:attachment, :lecture) end + + def validate_destroy_rights + @uploaded_file = UploadedFile.find(params[:id]) + owner = current_user == @uploaded_file.author + course_file_and_course_owner = (@uploaded_file.allowsUpload.class == Course) && (@uploaded_file.allowsUpload.creator_id == current_user.id) + lecture_file_and_lecture_owner = (@uploaded_file.allowsUpload.class == Lecture) && (@uploaded_file.allowsUpload.lecturer_id == current_user.id) + unless owner || course_file_and_course_owner || lecture_file_and_lecture_owner + redirect_to (uploaded_files_url), notice: "You can't delete this file." + end + end end diff --git a/app/views/courses/_filelist.html.erb b/app/views/courses/_filelist.html.erb index 60305b38..bd5b9f26 100644 --- a/app/views/courses/_filelist.html.erb +++ b/app/views/courses/_filelist.html.erb @@ -7,7 +7,7 @@
<% if @current_user == file.author || @current_user == @course.creator %> - + <%= link_to 'Delete File', file, method: :delete, class: "btn btn-sm btn-outline-danger", data: { confirm: 'Are you sure?' } %> <% end %>
diff --git a/config/routes.rb b/config/routes.rb index 86cef105..ccb5c87b 100755 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,5 +1,5 @@ Rails.application.routes.draw do - resources :uploaded_files, only: [:show, :index, :new, :create] + resources :uploaded_files, only: [:show, :index, :new, :create, :destroy] get "/courses/:course_id/lectures/current", to: "lectures#current", as: "current_lectures" post "/courses/:course_id/lectures/start_lecture", to: "lectures#start_lecture", as: "start_lecture" post "/courses/:course_id/lectures/join_lecture", to: "lectures#join_lecture", as: "join_lecture" diff --git a/spec/controllers/uploaded_files_controller_spec.rb b/spec/controllers/uploaded_files_controller_spec.rb index 825ad94e..29bf1dfc 100644 --- a/spec/controllers/uploaded_files_controller_spec.rb +++ b/spec/controllers/uploaded_files_controller_spec.rb @@ -48,10 +48,61 @@ end it "stores the file only if valid file provided" do - old_count = UploadedFile.count - @file["uploaded_file"].except!("attachment") - post :create, params: @file - expect(response).to_not redirect_to(uploaded_files_url) - expect(UploadedFile.count).to eq(old_count) + old_count = UploadedFile.count + @file["uploaded_file"].except!("attachment") + post :create, params: @file + expect(response).to_not redirect_to(uploaded_files_url) + expect(UploadedFile.count).to eq(old_count) + end + + context "having a file uploaded to a course as a student" do + before :each do + @student = FactoryBot.create(:user, :student) + @lecturer = FactoryBot.create(:user, :lecturer) + @course = FactoryBot.create(:course, creator: @lecturer) + @student_file = FactoryBot.create(:uploaded_file, author: @student, allowsUpload: @course) + sign_in @student + end + + it "can be deleted by the owner" do + expect { post :destroy, params: { id: @student_file[:id] } }.to change(UploadedFile, :count).by(-1) + end + + it "can be deleted by the course owner" do + sign_in @lecturer + expect { post :destroy, params: { id: @student_file[:id] } }.to change(UploadedFile, :count).by(-1) + end + + it "can't be deleted by someone else" do + @other_lecturer = FactoryBot.create(:user, :lecturer) + sign_in @other_lecturer + expect { post :destroy, params: { id: @student_file[:id] } }.to_not change(UploadedFile, :count) + end + end + + context "having a file uploaded to a lecture as a student" do + before :each do + @student = FactoryBot.create(:user, :student) + @lecturer = FactoryBot.create(:user, :lecturer) + @course = FactoryBot.create(:course, creator: @lecturer) + @lecture = FactoryBot.create(:lecture, lecturer: @lecturer, course: @course) + @student_file = FactoryBot.create(:uploaded_file, author: @student, allowsUpload: @lecture) + sign_in @student + end + + it "can be deleted by the owner" do + expect { post :destroy, params: { id: @student_file[:id] } }.to change(UploadedFile, :count).by(-1) end + + it "can be deleted by the lecture owner" do + sign_in @lecturer + expect { post :destroy, params: { id: @student_file[:id] } }.to change(UploadedFile, :count).by(-1) + end + + it "can't be deleted by someone else" do + @other_lecturer = FactoryBot.create(:user, :lecturer) + sign_in @other_lecturer + expect { post :destroy, params: { id: @student_file[:id] } }.to_not change(UploadedFile, :count) + end + end end diff --git a/spec/features/courses/show_course_spec.rb b/spec/features/courses/show_course_spec.rb index b8127c9e..8760ed13 100644 --- a/spec/features/courses/show_course_spec.rb +++ b/spec/features/courses/show_course_spec.rb @@ -111,5 +111,40 @@ expect(page.body).to eql @lecturer_file.data expect(page.response_headers["Content-Type"]).to eql @lecturer_file.content_type end + + it "should have delete links for every file" do + @student = FactoryBot.create(:user, :student) + @lecturer_file = FactoryBot.create(:uploaded_file, author: @lecturer, allowsUpload_id: @course.id, allowsUpload_type: "Course") + @student_file = FactoryBot.create(:uploaded_file, author: @student, allowsUpload_id: @course.id, allowsUpload_type: "Course") + visit(course_path(@course)) + @delete_link_student_file = find_link("Delete File", href: uploaded_file_path(@student_file)) + @delete_link_lecturer_file =find_link("Delete File", href: uploaded_file_path(@lecturer_file)) + expect { @delete_link_student_file.click }.to change(UploadedFile, :count).by(-1) + expect { @delete_link_lecturer_file.click }.to change(UploadedFile, :count).by(-1) + end + end + + context "being signed in as a student" do + before(:each) do + @lecturer = FactoryBot.create(:user, :lecturer) + @course = FactoryBot.create(:course, creator: @lecturer) + @lecture = FactoryBot.create(:lecture, lecturer: @lecturer, course: @course) + @student = FactoryBot.create(:user, :student) + @course.join_course(@student) + sign_in @student + end + + it "should only have delete links for his own files" do + @other_student = FactoryBot.create(:user, :student) + @other_student_file = FactoryBot.create(:uploaded_file, author: @other_student, allowsUpload_id: @course.id, allowsUpload_type: "Course") + @lecturer_file = FactoryBot.create(:uploaded_file, author: @lecturer, allowsUpload_id: @course.id, allowsUpload_type: "Course") + @student_file = FactoryBot.create(:uploaded_file, author: @student, allowsUpload_id: @course.id, allowsUpload_type: "Course") + visit(course_path(@course)) + expect(page).to have_selector("a[href='#{uploaded_file_path(@student_file)}'][data-method='delete']") + expect(page).to_not have_selector("a[href='#{uploaded_file_path(@lecturer_file)}'][data-method='delete']") + expect(page).to_not have_selector("a[href='#{uploaded_file_path(@other_student_file)}'][data-method='delete']") + @delete_link = find_link("Delete File", href: uploaded_file_path(@student_file)) + expect { @delete_link.click }.to change(UploadedFile, :count).by(-1) + end end end