Feature/#160 remove files course overview (#191)

* add prototype for filelist * add download link * Tests for #155, remove double tests for show_courses * Differntiate student and lecturer files and add scroll bar * Missing test case * Tests for #160 #156 * Implement Deleting of Course Files * Validate right to delete files from courses * rubocop * Remove duplicate section * Add lecture file delete validation * Update app/controllers/uploaded_files_controller.rb Co-Authored-By: Adrian Jost <[email protected]> * Update app/controllers/uploaded_files_controller.rb Co-Authored-By: Adrian Jost <[email protected]> * Update app/controllers/uploaded_files_controller.rb Co-Authored-By: Adrian Jost <[email protected]> * Update app/controllers/uploaded_files_controller.rb Co-Authored-By: Adrian Jost <[email protected]> * Update app/controllers/uploaded_files_controller.rb Co-Authored-By: Adrian Jost <[email protected]> * Update uploaded_files_controller.rb * change back variable scope * change variable scope Co-authored-by: Adrian Jost <[email protected]>
hpi-swt2 · Jan 16, 2020 · 9949c1f · 9949c1f
1 parent e7470a4
commit 9949c1f
Show file tree

Hide file tree

Showing 5 changed files with 115 additions and 7 deletions.
diff --git 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
@@ -7,7 +7,7 @@
       </div>
       <div>
         <% if @current_user == file.author || @current_user == @course.creator %>
-          <button class="btn btn-sm btn-outline-danger">TODO ADD DELETE ACTION</button>
+          <%=  link_to 'Delete File', file, method: :delete, class: "btn btn-sm btn-outline-danger", data: { confirm: 'Are you sure?' } %>
         <% end %>
       </div>
     </li>

diff --git 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
@@ -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
@@ -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