From 11a59aadfb8cf32de0808f8f8d98212cf9b21582 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 | 6 ++--- config/locales/en.yml | 4 ++-- test/abilities/abilities_test.rb | 3 +-- .../diary_entries_controller_test.rb | 24 +++++++++++++++++-- 8 files changed, 47 insertions(+), 18 deletions(-) diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index 7f47b578c..f5a40e603 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -41,7 +41,7 @@ class Ability can :create, :account_pd_declaration can :read, :dashboard 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 25de71f20..e5bf7eba4 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -298,7 +298,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 94876e72a..0f82da777 100644 --- a/app/controllers/diary_entries_controller.rb +++ b/app/controllers/diary_entries_controller.rb @@ -55,7 +55,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) @@ -65,8 +65,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) @entry = entries.find_by(:id => params[:id]) if @entry @title = t ".title", :user => params[:display_name], :title => @entry.title @@ -204,14 +203,24 @@ class DiaryEntriesController < ApplicationController def hide entry = DiaryEntry.find(params[: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[: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 089c7e6c6..8c6780606 100644 --- a/app/models/diary_entry.rb +++ b/app/models/diary_entry.rb @@ -36,6 +36,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 diff --git a/app/views/diary_entries/_diary_entry.html.erb b/app/views/diary_entries/_diary_entry.html.erb index 2d8243d1b..6f08e8ba5 100644 --- a/app/views/diary_entries/_diary_entry.html.erb +++ b/app/views/diary_entries/_diary_entry.html.erb @@ -33,12 +33,12 @@ <% 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") } %> + <%= link_to t(".delete_link"), hide_diary_entry_path(diary_entry.user, diary_entry), :method => :post, :data => { :confirm => t(".confirm") } %> <% else %> - <%= link_to t(".unhide_link"), unhide_diary_entry_path(diary_entry.user, diary_entry), :method => :post, :data => { :confirm => t(".confirm") } %> + <%= link_to t(".restore_link"), unhide_diary_entry_path(diary_entry.user, diary_entry), :method => :post, :data => { :confirm => t(".confirm") } %> <% end %>
    • <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 0320bc2a6..c7a41c58c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -606,8 +606,8 @@ en: other: "%{count} comments" no_comments: No comments edit_link: Edit this entry - hide_link: Hide this entry - unhide_link: Unhide this entry + delete_link: Delete this entry + restore_link: Restore this entry confirm: Confirm report: Report this entry diary_comment: diff --git a/test/abilities/abilities_test.rb b/test/abilities/abilities_test.rb index 99168375a..948930c0e 100644 --- a/test/abilities/abilities_test.rb +++ b/test/abilities/abilities_test.rb @@ -55,7 +55,7 @@ class UserAbilityTest < AbilityTest 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 @@ -64,7 +64,6 @@ class UserAbilityTest < AbilityTest 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 c96c433bf..56162e264 100644 --- a/test/controllers/diary_entries_controller_test.rb +++ b/test/controllers/diary_entries_controller_test.rb @@ -792,11 +792,21 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest assert DiaryEntry.find(diary_entry.id).visible # Now try as a normal user - session_for(user) + session_for(create(:user)) post hide_diary_entry_path(user, diary_entry) assert_redirected_to :controller => :errors, :action => :forbidden assert DiaryEntry.find(diary_entry.id).visible + # Now try as the author + session_for(user) + post hide_diary_entry_path(:display_name => user.display_name, :id => diary_entry) + assert_response :redirect + assert_redirected_to :action => :index, :display_name => user.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) @@ -823,11 +833,21 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest assert_not DiaryEntry.find(diary_entry.id).visible # Now try as a normal user - session_for(user) + session_for(create(:user)) post unhide_diary_entry_path(user, diary_entry) assert_redirected_to :controller => :errors, :action => :forbidden assert_not DiaryEntry.find(diary_entry.id).visible + # Now try as the author + session_for(user) + post unhide_diary_entry_path(:display_name => user.display_name, :id => diary_entry) + assert_response :redirect + assert_redirected_to :action => :index, :display_name => user.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)