Allow ability descriptions to be edited

app/controllers/abilities_controller.rb

@@ -1,25 +1,86 @@
 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_moderator, only: [:recalc]
+  before_action :verify_moderator, only: [:edit, :recalc, :update]
 
   def index
     @abilities = Ability.all
   end
 
   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
+    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)
+    else
+      render :edit, status: :bad_request
+    end
+  end
+
   def recalc
     @user.community_user.recalc_privileges!
     redirect_to user_privileges_url(@user.id)
   end
 
   private
 
+  def ability_update_params
+    params.require(:ability).permit(:description, :name)
+  end
+
+  # Actually update a given set of abilities network-wide
+  # @param ability [Ability] ability from which the push is initiated
+  # @param abilities [ActiveRecord::Relation<Ability>] 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?
+  end
+
   def set_user
     @user = if params[:for].present?
               User.where(id: params[:for]).first || @user

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?

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?
+    at_least_moderator?
+  end
+
   # Can the user edit tags?
   # @return [Boolean] check result
   def can_edit_tags?
@@ -218,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?

app/views/abilities/_form.html.erb

@@ -0,0 +1,49 @@
+<%= render 'posts/markdown_script' %>
+<%= render 'posts/image_upload' %>
+
+<% if @ability.errors.any? %>
+  <div class="notice is-danger">
+    There were some errors while saving this ability:
+
+    <ul>
+      <% @ability.errors.full_messages.each do |msg| %>
+        <li><%= msg %></li>
+      <% end %>
+    </ul>
+  </div>
+<% 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 %>
+           <div class="post-preview"></div>
+
+
+  <% if current_user&.can_push_to_network?(@ability) %>
+    <div class="form-group">
+      <div class="checkbox-setting">
+        <div class="checkbox-setting--desc">
+          <%= label_tag :network_push, 'Push to network', class: 'form-element' %>
+          <span class="form-caption">Copy to all communities (where the ability has not been edited)?</span>
+        </div>
+        <div class="checkbox-setting--value">
+          <%= check_box_tag :network_push, true, false, class: 'form-checkbox-element' %>
+        </div>
+      </div>
+    </div>
+  <% end %>
+
+  <%= 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 %>

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 %>
+
+<h1><%= title %></h1>
+
+<%= render 'form', submit_path: update_ability_path(id: @ability.internal_id) %>

app/views/abilities/show.html.erb

@@ -4,6 +4,9 @@
   <% else %>
     <a href="<%= abilities_url %>" class="button is-muted">Return to list</a>
   <% end %>
+  <% if current_user&.can_edit_abilities? %>
+    <%= link_to 'edit', edit_ability_path(id: @ability.internal_id), class: "button is-outlined is-muted" %>
+  <% end %>
 </div>
 <h1 class="h-m-t-0"><%= @ability.name %> Ability</h1>
 

config/locales/strings/en.abilities.yml

@@ -0,0 +1,10 @@
+en:
+  abilities:
+    #_form.html.erb
+    body_label: >
+      Description
+    unsaved_changes_confirmation: >
+      Any unsaved changes will be lost. Are you sure?
+    success:
+      update_generic: >
+        Ability successfully updated.

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

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

test/controllers/abilities_controller_test.rb

@@ -65,4 +65,80 @@ 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
+
+  test ':update should prevent invalid updates' do
+    ability = abilities(:everyone)
+    old_name = ability.name
+    user = users(:global_admin)
+
+    sign_in user
+
+    [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
+    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)
+      assert_redirected_to ability_url(id: ability.internal_id)
+
+      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,
+      network_push: network_push
+    }
+  end
 end

test/models/user_test.rb

@@ -85,11 +85,27 @@ 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
-    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)
+  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)
+    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