From 4b9593ddddbf6bcc261ffcc801ba2d85a1d06496 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sat, 21 Mar 2026 01:16:54 +0300 Subject: [PATCH 01/19] added scaffolding for editing abilities (global admin only for now) --- app/controllers/abilities_controller.rb | 22 ++++++++++++++++++++-- config/routes.rb | 8 +++++--- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/app/controllers/abilities_controller.rb b/app/controllers/abilities_controller.rb index 015cf6d96..aa31a321c 100644 --- a/app/controllers/abilities_controller.rb +++ b/app/controllers/abilities_controller.rb @@ -1,5 +1,7 @@ class AbilitiesController < ApplicationController + before_action :set_ability, only: [:show, :edit, :update] before_action :set_user + before_action :verify_global_admin, only: [:edit, :update] before_action :verify_moderator, only: [:recalc] def index @@ -8,11 +10,22 @@ def index def show @ability = Ability.where(internal_id: params[:id]).first - return not_found! if @ability.nil? - @your_ability = @user&.community_user&.privilege @ability.internal_id end + def edit end + + def update + update_params = {} + + if @ability.update(update_params) + + redirect_to ability_path(id: @ability.id) + else + render :edit, status: :bad_request + end + end + def recalc @user.community_user.recalc_privileges! redirect_to user_privileges_url(@user.id) @@ -20,6 +33,11 @@ def recalc private + def set_ability + @ability = Ability.where(internal_id: params[:id]).first + return not_found! unless @ability.present? + end + def set_user @user = if params[:for].present? User.where(id: params[:for]).first || @user diff --git a/config/routes.rb b/config/routes.rb index 05961caad..f42d48d1d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -344,9 +344,11 @@ end scope 'abilities' do - root to: 'abilities#index', as: :abilities - get 'recalc', to: 'abilities#recalc', as: :abilities_recalc - get ':id', to: 'abilities#show', as: :ability + root to: 'abilities#index', as: :abilities + get 'recalc', to: 'abilities#recalc', as: :abilities_recalc + get ':id', to: 'abilities#show', as: :ability + get ':id/edit', to: 'abilities#edit', as: :edit_ability + patch ':id/edit', to: 'abilities#update', as: :update_ability end scope 'donate' do From 11e8378d3bc183c07c33bce147006eddb25519a3 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sat, 21 Mar 2026 01:32:21 +0300 Subject: [PATCH 02/19] more scaffolding for editing abilities --- app/controllers/abilities_controller.rb | 2 +- app/views/abilities/_form.htm.erb | 15 +++++++++++++++ app/views/abilities/edit.html.erb | 14 ++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 app/views/abilities/_form.htm.erb create mode 100644 app/views/abilities/edit.html.erb diff --git a/app/controllers/abilities_controller.rb b/app/controllers/abilities_controller.rb index aa31a321c..b03a88f3f 100644 --- a/app/controllers/abilities_controller.rb +++ b/app/controllers/abilities_controller.rb @@ -13,7 +13,7 @@ def show @your_ability = @user&.community_user&.privilege @ability.internal_id end - def edit end + def edit; end def update update_params = {} diff --git a/app/views/abilities/_form.htm.erb b/app/views/abilities/_form.htm.erb new file mode 100644 index 000000000..9e2498451 --- /dev/null +++ b/app/views/abilities/_form.htm.erb @@ -0,0 +1,15 @@ +<%= render 'posts/markdown_script' %> + +<%= render 'posts/image_upload' %> + +<% if @ability.errors.any? %> +
+ There were some errors while saving this ability: + +
    + <% @ability.errors.full_messages.each do |msg| %> +
  • <%= msg %>
  • + <% end %> +
+
+<% end %> diff --git a/app/views/abilities/edit.html.erb b/app/views/abilities/edit.html.erb new file mode 100644 index 000000000..4a4eb762e --- /dev/null +++ b/app/views/abilities/edit.html.erb @@ -0,0 +1,14 @@ +<%# + "Instance variables: + @ability : Ability to edit +"%> + +<% + title = "Edit ability \"#{@ability.name}\"" +%> + +<% content_for :title, title %> + +

<%= title %>

+ +<%= render 'form', submit_path: update_ability_path(id: @ability.id) %> From e0eed3619c109ef2bca1a5852bdf57dc6a5b7d81 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sat, 21 Mar 2026 01:34:02 +0300 Subject: [PATCH 03/19] fixed duplication of set_ability in AbilitiesController#show --- app/controllers/abilities_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/abilities_controller.rb b/app/controllers/abilities_controller.rb index b03a88f3f..f043cea41 100644 --- a/app/controllers/abilities_controller.rb +++ b/app/controllers/abilities_controller.rb @@ -9,7 +9,6 @@ def index end def show - @ability = Ability.where(internal_id: params[:id]).first @your_ability = @user&.community_user&.privilege @ability.internal_id end From f1415aad7407a16d37968a932400f960934d415b Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sat, 21 Mar 2026 01:35:15 +0300 Subject: [PATCH 04/19] minor rubocop fix --- app/controllers/abilities_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/abilities_controller.rb b/app/controllers/abilities_controller.rb index f043cea41..44b6f406b 100644 --- a/app/controllers/abilities_controller.rb +++ b/app/controllers/abilities_controller.rb @@ -34,7 +34,7 @@ def recalc def set_ability @ability = Ability.where(internal_id: params[:id]).first - return not_found! unless @ability.present? + not_found! unless @ability.present? end def set_user From 8db0056f93649b0f5f644491454ef72c20754ee4 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sat, 21 Mar 2026 03:22:48 +0300 Subject: [PATCH 05/19] working ability edit implementation --- app/controllers/abilities_controller.rb | 14 +++++++---- app/models/user.rb | 6 +++++ app/views/abilities/_form.htm.erb | 15 ----------- app/views/abilities/_form.html.erb | 33 +++++++++++++++++++++++++ app/views/abilities/edit.html.erb | 2 +- app/views/abilities/show.html.erb | 3 +++ config/locales/strings/en.abilities.yml | 7 ++++++ 7 files changed, 59 insertions(+), 21 deletions(-) delete mode 100644 app/views/abilities/_form.htm.erb create mode 100644 app/views/abilities/_form.html.erb create mode 100644 config/locales/strings/en.abilities.yml diff --git a/app/controllers/abilities_controller.rb b/app/controllers/abilities_controller.rb index 44b6f406b..743d80b83 100644 --- a/app/controllers/abilities_controller.rb +++ b/app/controllers/abilities_controller.rb @@ -1,4 +1,6 @@ class AbilitiesController < ApplicationController + include DraftManagement + before_action :set_ability, only: [:show, :edit, :update] before_action :set_user before_action :verify_global_admin, only: [:edit, :update] @@ -15,11 +17,9 @@ def show def edit; end def update - update_params = {} - - if @ability.update(update_params) - - redirect_to ability_path(id: @ability.id) + if @ability.update(ability_update_params) + do_delete_draft(current_user, URI(request.referer || '').path) + redirect_to ability_path(id: @ability.internal_id) else render :edit, status: :bad_request end @@ -32,6 +32,10 @@ def recalc private + def ability_update_params + params.require(:ability).permit(:description) + end + def set_ability @ability = Ability.where(internal_id: params[:id]).first not_found! unless @ability.present? diff --git a/app/models/user.rb b/app/models/user.rb index 5e1607028..482d7dd46 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -155,6 +155,12 @@ def can_delete?(target) privilege?('flag_curate') && !target.deleted? end + # Can the user edit abilities? + # @return [Boolean] check result + def can_edit_abilities? + global_admin? + end + # Can the user edit tags? # @return [Boolean] check result def can_edit_tags? diff --git a/app/views/abilities/_form.htm.erb b/app/views/abilities/_form.htm.erb deleted file mode 100644 index 9e2498451..000000000 --- a/app/views/abilities/_form.htm.erb +++ /dev/null @@ -1,15 +0,0 @@ -<%= render 'posts/markdown_script' %> - -<%= render 'posts/image_upload' %> - -<% if @ability.errors.any? %> -
- There were some errors while saving this ability: - -
    - <% @ability.errors.full_messages.each do |msg| %> -
  • <%= msg %>
  • - <% end %> -
-
-<% end %> diff --git a/app/views/abilities/_form.html.erb b/app/views/abilities/_form.html.erb new file mode 100644 index 000000000..d754fa8a3 --- /dev/null +++ b/app/views/abilities/_form.html.erb @@ -0,0 +1,33 @@ +<%= render 'posts/markdown_script' %> +<%= render 'posts/image_upload' %> + +<% if @ability.errors.any? %> +
+ There were some errors while saving this ability: + +
    + <% @ability.errors.full_messages.each do |msg| %> +
  • <%= msg %>
  • + <% end %> +
+
+<% end %> + +<%= form_for @ability, url: submit_path do |f| %> + <%= render 'shared/body_field', + f: f, + field_name: :description, + field_label: t('abilities.body_label'), + post: @ability, + cur_length: @ability.description.length, + min_length: 15, + max_length: 30_000 %> +
+ <%= f.submit 'Save', class: 'button is-filled' %> + <%= link_to t('g.cancel').titlecase, + ability_path(id: @ability.internal_id), + class: 'button is-muted is-outlined is-danger js-cancel-edit', + data: { + question_body: t('abilities.unsaved_changes_confirmation') + } %> +<% end %> diff --git a/app/views/abilities/edit.html.erb b/app/views/abilities/edit.html.erb index 4a4eb762e..3bea65bc1 100644 --- a/app/views/abilities/edit.html.erb +++ b/app/views/abilities/edit.html.erb @@ -11,4 +11,4 @@

<%= title %>

-<%= render 'form', submit_path: update_ability_path(id: @ability.id) %> +<%= render 'form', submit_path: update_ability_path(id: @ability.internal_id) %> diff --git a/app/views/abilities/show.html.erb b/app/views/abilities/show.html.erb index a2b0f84dd..52ecbff80 100644 --- a/app/views/abilities/show.html.erb +++ b/app/views/abilities/show.html.erb @@ -4,6 +4,9 @@ <% else %> Return to list <% end %> + <% if current_user&.can_edit_abilities? %> + <%= link_to 'edit', edit_ability_path(id: @ability.internal_id), class: "button is-outlined is-muted" %> + <% end %>

<%= @ability.name %> Ability

diff --git a/config/locales/strings/en.abilities.yml b/config/locales/strings/en.abilities.yml new file mode 100644 index 000000000..7653ea600 --- /dev/null +++ b/config/locales/strings/en.abilities.yml @@ -0,0 +1,7 @@ +en: + abilities: + #_form.html.erb + body_label: > + Description + unsaved_changes_confirmation: > + Any unsaved changes will be lost. Are you sure? From 0f0d1119d23965f39985e2baee449703569782f4 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sat, 21 Mar 2026 03:43:43 +0300 Subject: [PATCH 06/19] added ability update success flash --- app/controllers/abilities_controller.rb | 1 + config/locales/strings/en.abilities.yml | 3 +++ 2 files changed, 4 insertions(+) diff --git a/app/controllers/abilities_controller.rb b/app/controllers/abilities_controller.rb index 743d80b83..6458d7495 100644 --- a/app/controllers/abilities_controller.rb +++ b/app/controllers/abilities_controller.rb @@ -19,6 +19,7 @@ def edit; end def update if @ability.update(ability_update_params) do_delete_draft(current_user, URI(request.referer || '').path) + flash[:success] = I18n.t('abilities.success.update_generic') redirect_to ability_path(id: @ability.internal_id) else render :edit, status: :bad_request diff --git a/config/locales/strings/en.abilities.yml b/config/locales/strings/en.abilities.yml index 7653ea600..da7abb350 100644 --- a/config/locales/strings/en.abilities.yml +++ b/config/locales/strings/en.abilities.yml @@ -5,3 +5,6 @@ en: Description unsaved_changes_confirmation: > Any unsaved changes will be lost. Are you sure? + success: + update_generic: > + Ability successfully updated. From 4e2d24f1e89a5671b39ec23ae206b41821bd16cf Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sat, 21 Mar 2026 04:45:02 +0300 Subject: [PATCH 07/19] sensitive abilities-related actions should require authentication --- app/controllers/abilities_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/abilities_controller.rb b/app/controllers/abilities_controller.rb index 6458d7495..30c0a4c3c 100644 --- a/app/controllers/abilities_controller.rb +++ b/app/controllers/abilities_controller.rb @@ -1,6 +1,7 @@ class AbilitiesController < ApplicationController include DraftManagement + before_action :authenticate_user!, except: [:index, :show] before_action :set_ability, only: [:show, :edit, :update] before_action :set_user before_action :verify_global_admin, only: [:edit, :update] From 1a1a03c787ecb1b07fb128d9dbcd802875c65a82 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sat, 21 Mar 2026 04:45:24 +0300 Subject: [PATCH 08/19] added tests for abilities controller's update action --- test/controllers/abilities_controller_test.rb | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/controllers/abilities_controller_test.rb b/test/controllers/abilities_controller_test.rb index a9ea0fe94..257564300 100644 --- a/test/controllers/abilities_controller_test.rb +++ b/test/controllers/abilities_controller_test.rb @@ -65,4 +65,40 @@ class AbilitiesControllerTest < ActionController::TestCase assert_not_nil assigns(:user) assert_not_nil assigns(:your_ability) end + + test ':update should require authentication' do + ability = abilities(:everyone) + try_update_ability(ability, description: 'anonymous') + assert_redirected_to_sign_in + end + + test ':update should correctly update abilities' do + ability = abilities(:everyone) + + users.each do |user| + description = "#{user.name}'s edit" + + sign_in user + try_update_ability(ability, description: description) + + if user.deleted? || user.community_user.deleted? + assert_redirected_to_sign_in + elsif user.can_edit_abilities? + assert_redirected_to ability_url(id: ability.internal_id) + assert_equal assigns(:ability).description, description + else + assert_response(:not_found) + end + end + end + + private + + # @param ability [Ability] ability to update + def try_update_ability(ability, **opts) + patch :update, params: { + ability: {}.merge(opts), + id: ability.internal_id + } + end end From ac149b3282a8199589ee38976c16c54703f7574a Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sat, 21 Mar 2026 17:20:43 +0300 Subject: [PATCH 09/19] allowed moderators to edit ability descriptions --- app/controllers/abilities_controller.rb | 3 +-- app/models/user.rb | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/controllers/abilities_controller.rb b/app/controllers/abilities_controller.rb index 30c0a4c3c..5a69e0819 100644 --- a/app/controllers/abilities_controller.rb +++ b/app/controllers/abilities_controller.rb @@ -4,8 +4,7 @@ class AbilitiesController < ApplicationController before_action :authenticate_user!, except: [:index, :show] before_action :set_ability, only: [:show, :edit, :update] before_action :set_user - before_action :verify_global_admin, only: [:edit, :update] - before_action :verify_moderator, only: [:recalc] + before_action :verify_moderator, only: [:edit, :recalc, :update] def index @abilities = Ability.all diff --git a/app/models/user.rb b/app/models/user.rb index 482d7dd46..71df901c3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -158,7 +158,7 @@ def can_delete?(target) # Can the user edit abilities? # @return [Boolean] check result def can_edit_abilities? - global_admin? + at_least_moderator? end # Can the user edit tags? From f5c4d276f4b26ea7476fe660bac51460029a27da Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sat, 21 Mar 2026 17:21:21 +0300 Subject: [PATCH 10/19] apparently, the schema hasn't been updated after the latest changes --- db/schema.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/db/schema.rb b/db/schema.rb index ac8ed1016..277027158 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -115,6 +115,7 @@ t.index ["community_id"], name: "index_categories_on_community_id" t.index ["default_filter_id"], name: "index_categories_on_default_filter_id" t.index ["license_id"], name: "index_categories_on_license_id" + t.index ["min_view_trust_level"], name: "index_categories_on_min_view_trust_level" t.index ["sequence"], name: "index_categories_on_sequence" t.index ["tag_set_id"], name: "index_categories_on_tag_set_id" end From 2201addc0e385256939c5df4f8bf235adbf4a8b6 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sat, 21 Mar 2026 20:52:23 +0300 Subject: [PATCH 11/19] made User#can_push_to_network? generic --- app/models/user.rb | 14 ++++++++++---- test/models/user_test.rb | 25 +++++++++++++++++++++---- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 71df901c3..ea240fcfa 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -224,11 +224,17 @@ def can_see_deleted_posts? privilege?('flag_curate') || false end - # Can the user push a given post type to network? - # @param post_type [PostType] type of the post to be pushed + # Can the user push a given target network-wide? + # @param target [Ability, PostType] target to be pushed # @return [Boolean] check result - def can_push_to_network?(post_type) - post_type.system? && (is_global_moderator || is_global_admin) + def can_push_to_network?(target) + return false unless global_moderator? || global_admin? + + if target.is_a?(PostType) + target.system? + else + true + end end # Can the user directly update a given post? diff --git a/test/models/user_test.rb b/test/models/user_test.rb index c92638738..5caa5e74b 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -85,11 +85,28 @@ class UserTest < ActiveSupport::TestCase assert_equal true, basic_user.can_update?(post, post_types(:free_edit)) end - test 'can_push_to_network? should determine if the user can push updates to network' do + test 'can_push_to_network? should correctly check network push permissions' do + global_mod = users(:global_moderator) + global_adm = users(:global_admin) + std_user = users(:standard_user) post_type = post_types(:help_doc) - assert_equal false, users(:standard_user).can_push_to_network?(post_type) - assert_equal true, users(:global_moderator).can_push_to_network?(post_type) - assert_equal true, users(:global_admin).can_push_to_network?(post_type) + network_users = [global_mod, global_adm] + + abilities.each do |ability| + assert_equal false, std_user.can_push_to_network?(ability) + + network_users.each do |user| + assert user.can_push_to_network?(ability) + end + end + + post_types.each do |type| + assert_equal false, std_user.can_push_to_network?(type) + + network_users.each do |user| + assert_equal type.system?, user.can_push_to_network?(type) + end + end end test 'community_user is based on context' do From 539d970afcbc498fefcec79a391e4dd10768cfb8 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sat, 21 Mar 2026 21:46:43 +0300 Subject: [PATCH 12/19] implemented network push for ability updates --- app/controllers/abilities_controller.rb | 41 ++++++++++++++++++++++++- app/views/abilities/_form.html.erb | 18 ++++++++++- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/app/controllers/abilities_controller.rb b/app/controllers/abilities_controller.rb index 5a69e0819..307f2c5ad 100644 --- a/app/controllers/abilities_controller.rb +++ b/app/controllers/abilities_controller.rb @@ -17,7 +17,18 @@ def show def edit; end def update - if @ability.update(ability_update_params) + if push_to_network?(@ability) + abilities = Ability.unscoped.where(internal_id: @ability.internal_id, + description: @ability.description) + + if do_update_network(@ability, abilities) + do_delete_draft(current_user, URI(request.referer || '').path) + flash[:success] = "#{helpers.pluralize(abilities.to_a.size, 'ability')} updated." + redirect_to ability_path(id: @ability.internal_id) + else + render :edit, status: :bad_request + end + elsif @ability.update(ability_update_params) do_delete_draft(current_user, URI(request.referer || '').path) flash[:success] = I18n.t('abilities.success.update_generic') redirect_to ability_path(id: @ability.internal_id) @@ -37,6 +48,34 @@ def ability_update_params params.require(:ability).permit(:description) end + # Actually update a given set of abilities network-wide + # @param ability [Ability] ability from which the push is initiated + # @param abilities [ActiveRecord::Relation] network abilities to update + # @return [Boolean] status of the operation + def do_update_network(ability, abilities) + Ability.transaction do + abilities.each do |network_ability| + unless network_ability.update(ability_update_params) + ability.errors.merge!(network_ability.errors) + raise ActiveRecord::Rollback + end + end + true + rescue + false + end + end + + # Should push to network? + # @param ability [Ability] ability to check + # @return [Boolean] check result + def push_to_network?(ability) + return false unless params[:network_push] == 'true' + return false unless current_user.present? + + current_user.can_push_to_network?(ability) + end + def set_ability @ability = Ability.where(internal_id: params[:id]).first not_found! unless @ability.present? diff --git a/app/views/abilities/_form.html.erb b/app/views/abilities/_form.html.erb index d754fa8a3..bbee25571 100644 --- a/app/views/abilities/_form.html.erb +++ b/app/views/abilities/_form.html.erb @@ -22,7 +22,23 @@ cur_length: @ability.description.length, min_length: 15, max_length: 30_000 %> -
+
+ + + <% if current_user&.can_push_to_network?(@ability) %> +
+
+
+ <%= label_tag :network_push, 'Push to network', class: 'form-element' %> + Copy to all communities (where the ability has not been edited)? +
+
+ <%= check_box_tag :network_push, true, false, class: 'form-checkbox-element' %> +
+
+
+ <% end %> + <%= f.submit 'Save', class: 'button is-filled' %> <%= link_to t('g.cancel').titlecase, ability_path(id: @ability.internal_id), From 5f069e409aa4785bffe0f68686f4fba2450423f9 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sat, 21 Mar 2026 21:51:24 +0300 Subject: [PATCH 13/19] minor rubocop fix --- test/models/user_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 5caa5e74b..1acebcd7c 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -89,7 +89,6 @@ class UserTest < ActiveSupport::TestCase global_mod = users(:global_moderator) global_adm = users(:global_admin) std_user = users(:standard_user) - post_type = post_types(:help_doc) network_users = [global_mod, global_adm] abilities.each do |ability| From c52793a9bcf7b3dabbd20b259bfe79f82ab6f950 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sun, 22 Mar 2026 00:08:58 +0300 Subject: [PATCH 14/19] added test for ability update network push --- test/controllers/abilities_controller_test.rb | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/test/controllers/abilities_controller_test.rb b/test/controllers/abilities_controller_test.rb index 257564300..b3d1ea80c 100644 --- a/test/controllers/abilities_controller_test.rb +++ b/test/controllers/abilities_controller_test.rb @@ -92,13 +92,34 @@ class AbilitiesControllerTest < ActionController::TestCase end end + test ':update should allow global mods & admins to network push' do + ability = abilities(:everyone) + + users.select { |u| u.can_push_to_network?(ability) }.each do |user| + description = "#{user.name}'s edit" + + sign_in user + try_update_ability(ability, description: description, network_push: true) + + network_abilities = Ability.unscoped.where(internal_id: ability.internal_id) + assert network_abilities.any? + + network_abilities.each do |network_ability| + assert_equal network_ability.description, description + end + end + end + private # @param ability [Ability] ability to update def try_update_ability(ability, **opts) + network_push = opts.delete(:network_push) || false + patch :update, params: { ability: {}.merge(opts), - id: ability.internal_id + id: ability.internal_id, + network_push: network_push } end end From b4209abe982c2ada7a5ce575cf6ad648d0173446 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sun, 22 Mar 2026 00:40:22 +0300 Subject: [PATCH 15/19] ability edit form should account for ability descriptions being optional --- app/views/abilities/_form.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/abilities/_form.html.erb b/app/views/abilities/_form.html.erb index bbee25571..435f1f89c 100644 --- a/app/views/abilities/_form.html.erb +++ b/app/views/abilities/_form.html.erb @@ -19,7 +19,7 @@ field_name: :description, field_label: t('abilities.body_label'), post: @ability, - cur_length: @ability.description.length, + cur_length: @ability.description&.length, min_length: 15, max_length: 30_000 %>
From 3c789a2d297f0dc4f5a4ae95ff07a9a57e049518 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sun, 22 Mar 2026 00:40:37 +0300 Subject: [PATCH 16/19] added presence validation for ability names --- app/models/ability.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/ability.rb b/app/models/ability.rb index b377084c8..228b89230 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -2,6 +2,7 @@ class Ability < ApplicationRecord include CommunityRelated include AbilitiesHelper + validates :name, presence: true validates :internal_id, uniqueness: { scope: [:community_id], case_sensitive: false } def manual? From b05f3d71e0157bfd6eaded9c79e025c79a4d961b Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sun, 22 Mar 2026 00:41:04 +0300 Subject: [PATCH 17/19] technically allowed ability names to be updated as well (no UI for now) --- app/controllers/abilities_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/abilities_controller.rb b/app/controllers/abilities_controller.rb index 307f2c5ad..c2d23eb79 100644 --- a/app/controllers/abilities_controller.rb +++ b/app/controllers/abilities_controller.rb @@ -45,7 +45,7 @@ def recalc private def ability_update_params - params.require(:ability).permit(:description) + params.require(:ability).permit(:description, :name) end # Actually update a given set of abilities network-wide From 7d7986bf8c2cfbf0f4016a7770ba5efbea6f8b3c Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sun, 22 Mar 2026 00:41:29 +0300 Subject: [PATCH 18/19] added test for rejecting invalid ability updates --- test/controllers/abilities_controller_test.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/controllers/abilities_controller_test.rb b/test/controllers/abilities_controller_test.rb index b3d1ea80c..225de6459 100644 --- a/test/controllers/abilities_controller_test.rb +++ b/test/controllers/abilities_controller_test.rb @@ -92,6 +92,20 @@ class AbilitiesControllerTest < ActionController::TestCase end end + test ':update should prevent invalid updates' do + ability = abilities(:everyone) + user = users(:global_admin) + + old_name = ability.name + + sign_in user + try_update_ability(ability, description: 'valid', name: '') + assert_response(:bad_request) + + ability.reload + assert_equal ability.name, old_name + end + test ':update should allow global mods & admins to network push' do ability = abilities(:everyone) @@ -100,6 +114,7 @@ class AbilitiesControllerTest < ActionController::TestCase sign_in user try_update_ability(ability, description: description, network_push: true) + assert_redirected_to ability_url(id: ability.internal_id) network_abilities = Ability.unscoped.where(internal_id: ability.internal_id) assert network_abilities.any? From 99bb8918fa79c25deceef4aea48a0beb0412be54 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sun, 22 Mar 2026 00:51:13 +0300 Subject: [PATCH 19/19] improved invalid ability updates test coverage --- test/controllers/abilities_controller_test.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/test/controllers/abilities_controller_test.rb b/test/controllers/abilities_controller_test.rb index 225de6459..da60b80b1 100644 --- a/test/controllers/abilities_controller_test.rb +++ b/test/controllers/abilities_controller_test.rb @@ -94,16 +94,20 @@ class AbilitiesControllerTest < ActionController::TestCase test ':update should prevent invalid updates' do ability = abilities(:everyone) - user = users(:global_admin) - old_name = ability.name + user = users(:global_admin) sign_in user - try_update_ability(ability, description: 'valid', name: '') - assert_response(:bad_request) - ability.reload - assert_equal ability.name, old_name + [false, true].each do |network_push| + try_update_ability(ability, description: 'valid', + name: '', + network_push: network_push) + assert_response(:bad_request) + + ability.reload + assert_equal ability.name, old_name + end end test ':update should allow global mods & admins to network push' do