From 28acf21416a550d2eb4387434e225ddfba8b65a6 Mon Sep 17 00:00:00 2001 From: Katsuya HIDAKA Date: Wed, 14 May 2025 10:48:37 +0900 Subject: Improve method and variable naming for clarity and consistency --- app/controllers/reactions_controller.rb | 2 +- app/helpers/reactions_helper.rb | 10 ++++----- lib/redmine/reaction.rb | 8 +++---- test/unit/lib/redmine/reaction_test.rb | 28 ++++++++++++------------- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/app/controllers/reactions_controller.rb b/app/controllers/reactions_controller.rb index f768f939d..71b37e5f8 100644 --- a/app/controllers/reactions_controller.rb +++ b/app/controllers/reactions_controller.rb @@ -60,6 +60,6 @@ class ReactionsController < ApplicationController end def authorize_reactable - render_403 unless Redmine::Reaction.writable?(@object, User.current) + render_403 unless Redmine::Reaction.editable?(@object, User.current) end end diff --git a/app/helpers/reactions_helper.rb b/app/helpers/reactions_helper.rb index 911da7127..623a7a0e8 100644 --- a/app/helpers/reactions_helper.rb +++ b/app/helpers/reactions_helper.rb @@ -22,19 +22,19 @@ module ReactionsHelper DISPLAY_REACTION_USERS_LIMIT = 10 def reaction_button(object) - return unless Redmine::Reaction.visible?(object, User.current) + return unless Redmine::Reaction.viewable?(object, User.current) detail = object.reaction_detail - reaction = detail.user_reaction + user_reaction = detail.user_reaction count = detail.reaction_count visible_user_names = detail.visible_users.take(DISPLAY_REACTION_USERS_LIMIT).map(&:name) tooltip = build_reaction_tooltip(visible_user_names, count) - if Redmine::Reaction.writable?(object, User.current) - if reaction&.persisted? - reaction_button_reacted(object, reaction, count, tooltip) + if Redmine::Reaction.editable?(object, User.current) + if user_reaction.present? + reaction_button_reacted(object, user_reaction, count, tooltip) else reaction_button_not_reacted(object, count, tooltip) end diff --git a/lib/redmine/reaction.rb b/lib/redmine/reaction.rb index b6f2bf075..17797a7d7 100644 --- a/lib/redmine/reaction.rb +++ b/lib/redmine/reaction.rb @@ -22,14 +22,14 @@ module Redmine # Types of objects that can have reactions REACTABLE_TYPES = %w(Journal Issue Message News Comment) - # Returns true if the user can view the reaction information of the object - def self.visible?(object, user = User.current) + # Returns true if the user can view the reaction of the object + def self.viewable?(object, user = User.current) Setting.reactions_enabled? && object.visible?(user) end # Returns true if the user can add/remove a reaction to/from the object - def self.writable?(object, user = User.current) - user.logged? && visible?(object, user) && object&.project&.active? + def self.editable?(object, user = User.current) + user.logged? && viewable?(object, user) && object&.project&.active? end module Reactable diff --git a/test/unit/lib/redmine/reaction_test.rb b/test/unit/lib/redmine/reaction_test.rb index 7b2bd574b..45475a163 100644 --- a/test/unit/lib/redmine/reaction_test.rb +++ b/test/unit/lib/redmine/reaction_test.rb @@ -124,32 +124,32 @@ class Redmine::ReactionTest < ActiveSupport::TestCase ), comment1.reaction_detail end - test 'visible? returns true when reactions are enabled and object is visible to user' do + test 'viewable? returns true when reactions are enabled and object is visible to user' do object = issues(:issues_007) user = users(:users_002) - assert Redmine::Reaction.visible?(object, user) + assert Redmine::Reaction.viewable?(object, user) end - test 'visible? returns false when reactions are disabled' do + test 'viewable? returns false when reactions are disabled' do Setting.reactions_enabled = '0' object = issues(:issues_007) user = users(:users_002) - assert_not Redmine::Reaction.visible?(object, user) + assert_not Redmine::Reaction.viewable?(object, user) end - test 'visible? returns false when object is not visible to user' do + test 'viewable? returns false when object is not visible to user' do object = issues(:issues_007) user = users(:users_002) object.expects(:visible?).with(user).returns(false) - assert_not Redmine::Reaction.visible?(object, user) + assert_not Redmine::Reaction.viewable?(object, user) end - test 'writable? returns true for various reactable objects when user is logged in, object is visible, and project is active' do + test 'editable? returns true for various reactable objects when user is logged in, object is visible, and project is active' do reactable_objects = { issue: issues(:issues_007), message: messages(:messages_001), @@ -160,30 +160,30 @@ class Redmine::ReactionTest < ActiveSupport::TestCase user = users(:users_002) reactable_objects.each do |type, object| - assert Redmine::Reaction.writable?(object, user), "Expected writable? to return true for #{type}" + assert Redmine::Reaction.editable?(object, user), "Expected editable? to return true for #{type}" end end - test 'writable? returns false when user is not logged in' do + test 'editable? returns false when user is not logged in' do object = issues(:issues_007) user = User.anonymous - assert_not Redmine::Reaction.writable?(object, user) + assert_not Redmine::Reaction.editable?(object, user) end - test 'writable? returns false when project is inactive' do + test 'editable? returns false when project is inactive' do object = issues(:issues_007) user = users(:users_002) object.project.update!(status: Project::STATUS_ARCHIVED) - assert_not Redmine::Reaction.writable?(object, user) + assert_not Redmine::Reaction.editable?(object, user) end - test 'writable? returns false when project is closed' do + test 'editable? returns false when project is closed' do object = issues(:issues_007) user = users(:users_002) object.project.update!(status: Project::STATUS_CLOSED) - assert_not Redmine::Reaction.writable?(object, user) + assert_not Redmine::Reaction.editable?(object, user) end end -- 2.49.0