From b8ec7355e6dedc560c9c76b891c36285899fd92d Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Sun, 25 Feb 2024 11:02:15 +0000 Subject: [PATCH] Allow users to delete their own diary entries Fixes #4175 --- app/abilities/ability.rb | 2 +- app/controllers/application_controller.rb | 2 +- app/controllers/diary_entries_controller.rb | 23 +++++--- app/models/diary_entry.rb | 1 + app/views/diary_entries/_diary_entry.html.erb | 2 +- test/abilities/user_ability_test.rb | 3 +- .../diary_entries_controller_test.rb | 56 +++++++++++++------ 7 files changed, 59 insertions(+), 30 deletions(-) diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index 4962f8a8c..6a8a9b7de 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -43,7 +43,7 @@ class Ability can :read, :dashboard can [:read, :update], [:preferences, :profile] can [:create, :subscribe, :unsubscribe], DiaryEntry - can :update, DiaryEntry, :user => user + can [:update, :hide, :unhide], DiaryEntry, :user => user can [:create], DiaryComment can [:show, :create, :destroy], Follow can [:read, :create, :destroy], Message diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index c096bbb74..1440f67eb 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -294,7 +294,7 @@ class ApplicationController < ActionController::Base Ability.new(current_user) end - def deny_access(_exception) + def deny_access(_exception = nil) if current_user set_locale respond_to do |format| diff --git a/app/controllers/diary_entries_controller.rb b/app/controllers/diary_entries_controller.rb index 8a97aac07..697bc1afd 100644 --- a/app/controllers/diary_entries_controller.rb +++ b/app/controllers/diary_entries_controller.rb @@ -50,7 +50,7 @@ class DiaryEntriesController < ApplicationController end end - entries = entries.visible unless can? :unhide, DiaryEntry + entries = entries.visible_to(current_user) @params = params.permit(:display_name, :friends, :nearby, :language) @@ -58,8 +58,7 @@ class DiaryEntriesController < ApplicationController end def show - entries = @user.diary_entries - entries = entries.visible unless can? :unhide, DiaryEntry + entries = @user.diary_entries.visible_to(current_user) @diary_entry = entries.find_by(:id => params[:id]) if @diary_entry @title = t ".title", :user => params[:display_name], :title => @diary_entry.title @@ -188,14 +187,24 @@ class DiaryEntriesController < ApplicationController def hide entry = DiaryEntry.find(params.expect(:id)) - entry.update(:visible => false) - redirect_to :action => "index", :display_name => entry.user.display_name + + if can?(:hide, entry) + entry.update(:visible => false) + redirect_to :action => "index", :display_name => entry.user.display_name + else + deny_access + end end def unhide entry = DiaryEntry.find(params.expect(:id)) - entry.update(:visible => true) - redirect_to :action => "index", :display_name => entry.user.display_name + + if can?(:unhide, entry) + entry.update(:visible => true) + redirect_to :action => "index", :display_name => entry.user.display_name + else + deny_access + end end private diff --git a/app/models/diary_entry.rb b/app/models/diary_entry.rb index 924ef41f8..4ec5ce6be 100644 --- a/app/models/diary_entry.rb +++ b/app/models/diary_entry.rb @@ -38,6 +38,7 @@ class DiaryEntry < ApplicationRecord has_many :subscribers, :through => :subscriptions, :source => :user scope :visible, -> { where(:visible => true) } + scope :visible_to, ->(user) { where(:visible => true).or(where(:user => user)) unless user&.moderator? || user&.administrator? } validates :title, :presence => true, :length => 1..255, :characters => true validates :body, :presence => true, :characters => true, :length => 1..262144 diff --git a/app/views/diary_entries/_diary_entry.html.erb b/app/views/diary_entries/_diary_entry.html.erb index fb598603b..b7c046ddd 100644 --- a/app/views/diary_entries/_diary_entry.html.erb +++ b/app/views/diary_entries/_diary_entry.html.erb @@ -43,7 +43,7 @@ <% end %> - <% if can? :hide, DiaryEntry %> + <% if can? :hide, diary_entry %>
  • <% if diary_entry.visible %> <%= link_to t(".hide_link"), hide_diary_entry_path(diary_entry.user, diary_entry), :method => :post, :data => { :confirm => t(".confirm") } %> diff --git a/test/abilities/user_ability_test.rb b/test/abilities/user_ability_test.rb index f1c69567c..da6b71ef3 100644 --- a/test/abilities/user_ability_test.rb +++ b/test/abilities/user_ability_test.rb @@ -6,7 +6,7 @@ class UserAbilityTest < ActiveSupport::TestCase test "Diary permissions" do ability = Ability.new create(:user) - [:index, :rss, :show, :create, :edit, :subscribe, :unsubscribe].each do |action| + [:index, :rss, :show, :create, :edit, :subscribe, :unsubscribe, :hide, :unhide].each do |action| assert ability.can?(action, DiaryEntry), "should be able to #{action} DiaryEntries" end @@ -15,7 +15,6 @@ class UserAbilityTest < ActiveSupport::TestCase end [:hide, :unhide].each do |action| - assert ability.cannot?(action, DiaryEntry), "should not be able to #{action} DiaryEntries" assert ability.cannot?(action, DiaryComment), "should not be able to #{action} DiaryComment" end diff --git a/test/controllers/diary_entries_controller_test.rb b/test/controllers/diary_entries_controller_test.rb index efdc59a68..4f59e580d 100644 --- a/test/controllers/diary_entries_controller_test.rb +++ b/test/controllers/diary_entries_controller_test.rb @@ -784,24 +784,34 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest end def test_hide - user = create(:user) - diary_entry = create(:diary_entry, :user => user) + author = create(:user) + diary_entry = create(:diary_entry, :user => author) # Try without logging in - post hide_diary_entry_path(user, diary_entry) + post hide_diary_entry_path(author, diary_entry) assert_response :forbidden assert DiaryEntry.find(diary_entry.id).visible # Now try as a normal user - session_for(user) - post hide_diary_entry_path(user, diary_entry) + session_for(create(:user)) + post hide_diary_entry_path(author, diary_entry) assert_redirected_to :controller => :errors, :action => :forbidden assert DiaryEntry.find(diary_entry.id).visible + # Now try as the author + session_for(author) + post hide_diary_entry_path(:display_name => author.display_name, :id => diary_entry) + assert_response :redirect + assert_redirected_to :action => :index, :display_name => author.display_name + assert_not DiaryEntry.find(diary_entry.id).visible + + # Reset + diary_entry.reload.update(:visible => true) + # Now try as a moderator session_for(create(:moderator_user)) - post hide_diary_entry_path(user, diary_entry) - assert_redirected_to :action => :index, :display_name => user.display_name + post hide_diary_entry_path(author, diary_entry) + assert_redirected_to :action => :index, :display_name => author.display_name assert_not DiaryEntry.find(diary_entry.id).visible # Reset @@ -809,30 +819,40 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest # Finally try as an administrator session_for(create(:administrator_user)) - post hide_diary_entry_path(user, diary_entry) - assert_redirected_to :action => :index, :display_name => user.display_name + post hide_diary_entry_path(author, diary_entry) + assert_redirected_to :action => :index, :display_name => author.display_name assert_not DiaryEntry.find(diary_entry.id).visible end def test_unhide - user = create(:user) + author = create(:user) # Try without logging in - diary_entry = create(:diary_entry, :user => user, :visible => false) - post unhide_diary_entry_path(user, diary_entry) + diary_entry = create(:diary_entry, :user => author, :visible => false) + post unhide_diary_entry_path(author, diary_entry) assert_response :forbidden assert_not DiaryEntry.find(diary_entry.id).visible # Now try as a normal user - session_for(user) - post unhide_diary_entry_path(user, diary_entry) + session_for(create(:user)) + post unhide_diary_entry_path(author, diary_entry) assert_redirected_to :controller => :errors, :action => :forbidden assert_not DiaryEntry.find(diary_entry.id).visible + # Now try as the author + session_for(author) + post unhide_diary_entry_path(:display_name => author.display_name, :id => diary_entry) + assert_response :redirect + assert_redirected_to :action => :index, :display_name => author.display_name + assert DiaryEntry.find(diary_entry.id).visible + + # Reset + diary_entry.reload.update(:visible => true) + # Now try as a moderator session_for(create(:moderator_user)) - post unhide_diary_entry_path(user, diary_entry) - assert_redirected_to :action => :index, :display_name => user.display_name + post unhide_diary_entry_path(author, diary_entry) + assert_redirected_to :action => :index, :display_name => author.display_name assert DiaryEntry.find(diary_entry.id).visible # Reset @@ -840,8 +860,8 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest # Finally try as an administrator session_for(create(:administrator_user)) - post unhide_diary_entry_path(user, diary_entry) - assert_redirected_to :action => :index, :display_name => user.display_name + post unhide_diary_entry_path(author, diary_entry) + assert_redirected_to :action => :index, :display_name => author.display_name assert DiaryEntry.find(diary_entry.id).visible end -- 2.47.3