Skip to content

Commit e88f0ab

Browse files
ArtOfCode-ArtOfCode-
authored
Merge pull request #2031 from codidact/0valt/abilities-edit
2 parents de29259 + 99bb891 commit e88f0ab

11 files changed

Lines changed: 261 additions & 16 deletions

File tree

‎app/controllers/abilities_controller.rb‎

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,86 @@
11
class AbilitiesController < ApplicationController
2+
include DraftManagement
3+
4+
before_action :authenticate_user!, except: [:index, :show]
5+
before_action :set_ability, only: [:show, :edit, :update]
26
before_action :set_user
3-
before_action :verify_moderator, only: [:recalc]
7+
before_action :verify_moderator, only: [:edit, :recalc, :update]
48

59
def index
610
@abilities = Ability.all
711
end
812

913
def show
10-
@ability = Ability.where(internal_id: params[:id]).first
11-
return not_found! if @ability.nil?
12-
1314
@your_ability = @user&.community_user&.privilege @ability.internal_id
1415
end
1516

17+
def edit; end
18+
19+
def update
20+
if push_to_network?(@ability)
21+
abilities = Ability.unscoped.where(internal_id: @ability.internal_id,
22+
description: @ability.description)
23+
24+
if do_update_network(@ability, abilities)
25+
do_delete_draft(current_user, URI(request.referer || '').path)
26+
flash[:success] = "#{helpers.pluralize(abilities.to_a.size, 'ability')} updated."
27+
redirect_to ability_path(id: @ability.internal_id)
28+
else
29+
render :edit, status: :bad_request
30+
end
31+
elsif @ability.update(ability_update_params)
32+
do_delete_draft(current_user, URI(request.referer || '').path)
33+
flash[:success] = I18n.t('abilities.success.update_generic')
34+
redirect_to ability_path(id: @ability.internal_id)
35+
else
36+
render :edit, status: :bad_request
37+
end
38+
end
39+
1640
def recalc
1741
@user.community_user.recalc_privileges!
1842
redirect_to user_privileges_url(@user.id)
1943
end
2044

2145
private
2246

47+
def ability_update_params
48+
params.require(:ability).permit(:description, :name)
49+
end
50+
51+
# Actually update a given set of abilities network-wide
52+
# @param ability [Ability] ability from which the push is initiated
53+
# @param abilities [ActiveRecord::Relation<Ability>] network abilities to update
54+
# @return [Boolean] status of the operation
55+
def do_update_network(ability, abilities)
56+
Ability.transaction do
57+
abilities.each do |network_ability|
58+
unless network_ability.update(ability_update_params)
59+
ability.errors.merge!(network_ability.errors)
60+
raise ActiveRecord::Rollback
61+
end
62+
end
63+
true
64+
rescue
65+
false
66+
end
67+
end
68+
69+
# Should push to network?
70+
# @param ability [Ability] ability to check
71+
# @return [Boolean] check result
72+
def push_to_network?(ability)
73+
return false unless params[:network_push] == 'true'
74+
return false unless current_user.present?
75+
76+
current_user.can_push_to_network?(ability)
77+
end
78+
79+
def set_ability
80+
@ability = Ability.where(internal_id: params[:id]).first
81+
not_found! unless @ability.present?
82+
end
83+
2384
def set_user
2485
@user = if params[:for].present?
2586
User.where(id: params[:for]).first || @user

‎app/models/ability.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ class Ability < ApplicationRecord
22
include CommunityRelated
33
include AbilitiesHelper
44

5+
validates :name, presence: true
56
validates :internal_id, uniqueness: { scope: [:community_id], case_sensitive: false }
67

78
def manual?

‎app/models/user.rb‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,12 @@ def can_delete?(target)
155155
privilege?('flag_curate') && !target.deleted?
156156
end
157157

158+
# Can the user edit abilities?
159+
# @return [Boolean] check result
160+
def can_edit_abilities?
161+
at_least_moderator?
162+
end
163+
158164
# Can the user edit tags?
159165
# @return [Boolean] check result
160166
def can_edit_tags?
@@ -218,11 +224,17 @@ def can_see_deleted_posts?
218224
privilege?('flag_curate') || false
219225
end
220226

221-
# Can the user push a given post type to network?
222-
# @param post_type [PostType] type of the post to be pushed
227+
# Can the user push a given target network-wide?
228+
# @param target [Ability, PostType] target to be pushed
223229
# @return [Boolean] check result
224-
def can_push_to_network?(post_type)
225-
post_type.system? && (is_global_moderator || is_global_admin)
230+
def can_push_to_network?(target)
231+
return false unless global_moderator? || global_admin?
232+
233+
if target.is_a?(PostType)
234+
target.system?
235+
else
236+
true
237+
end
226238
end
227239

228240
# Can the user directly update a given post?

‎app/views/abilities/_form.html.erb‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<%= render 'posts/markdown_script' %>
2+
<%= render 'posts/image_upload' %>
3+
4+
<% if @ability.errors.any? %>
5+
<div class="notice is-danger">
6+
There were some errors while saving this ability:
7+
8+
<ul>
9+
<% @ability.errors.full_messages.each do |msg| %>
10+
<li><%= msg %></li>
11+
<% end %>
12+
</ul>
13+
</div>
14+
<% end %>
15+
16+
<%= form_for @ability, url: submit_path do |f| %>
17+
<%= render 'shared/body_field',
18+
f: f,
19+
field_name: :description,
20+
field_label: t('abilities.body_label'),
21+
post: @ability,
22+
cur_length: @ability.description&.length,
23+
min_length: 15,
24+
max_length: 30_000 %>
25+
<div class="post-preview"></div>
26+
27+
28+
<% if current_user&.can_push_to_network?(@ability) %>
29+
<div class="form-group">
30+
<div class="checkbox-setting">
31+
<div class="checkbox-setting--desc">
32+
<%= label_tag :network_push, 'Push to network', class: 'form-element' %>
33+
<span class="form-caption">Copy to all communities (where the ability has not been edited)?</span>
34+
</div>
35+
<div class="checkbox-setting--value">
36+
<%= check_box_tag :network_push, true, false, class: 'form-checkbox-element' %>
37+
</div>
38+
</div>
39+
</div>
40+
<% end %>
41+
42+
<%= f.submit 'Save', class: 'button is-filled' %>
43+
<%= link_to t('g.cancel').titlecase,
44+
ability_path(id: @ability.internal_id),
45+
class: 'button is-muted is-outlined is-danger js-cancel-edit',
46+
data: {
47+
question_body: t('abilities.unsaved_changes_confirmation')
48+
} %>
49+
<% end %>

‎app/views/abilities/edit.html.erb‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<%#
2+
"Instance variables:
3+
@ability : Ability to edit
4+
"%>
5+
6+
<%
7+
title = "Edit ability \"#{@ability.name}\""
8+
%>
9+
10+
<% content_for :title, title %>
11+
12+
<h1><%= title %></h1>
13+
14+
<%= render 'form', submit_path: update_ability_path(id: @ability.internal_id) %>

‎app/views/abilities/show.html.erb‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
<% else %>
55
<a href="<%= abilities_url %>" class="button is-muted">Return to list</a>
66
<% end %>
7+
<% if current_user&.can_edit_abilities? %>
8+
<%= link_to 'edit', edit_ability_path(id: @ability.internal_id), class: "button is-outlined is-muted" %>
9+
<% end %>
710
</div>
811
<h1 class="h-m-t-0"><%= @ability.name %> Ability</h1>
912

‎config/locales/strings/en.abilities.yml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
en:
2+
abilities:
3+
#_form.html.erb
4+
body_label: >
5+
Description
6+
unsaved_changes_confirmation: >
7+
Any unsaved changes will be lost. Are you sure?
8+
success:
9+
update_generic: >
10+
Ability successfully updated.

‎config/routes.rb‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,9 +344,11 @@
344344
end
345345

346346
scope 'abilities' do
347-
root to: 'abilities#index', as: :abilities
348-
get 'recalc', to: 'abilities#recalc', as: :abilities_recalc
349-
get ':id', to: 'abilities#show', as: :ability
347+
root to: 'abilities#index', as: :abilities
348+
get 'recalc', to: 'abilities#recalc', as: :abilities_recalc
349+
get ':id', to: 'abilities#show', as: :ability
350+
get ':id/edit', to: 'abilities#edit', as: :edit_ability
351+
patch ':id/edit', to: 'abilities#update', as: :update_ability
350352
end
351353

352354
scope 'donate' do

‎db/schema.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@
115115
t.index ["community_id"], name: "index_categories_on_community_id"
116116
t.index ["default_filter_id"], name: "index_categories_on_default_filter_id"
117117
t.index ["license_id"], name: "index_categories_on_license_id"
118+
t.index ["min_view_trust_level"], name: "index_categories_on_min_view_trust_level"
118119
t.index ["sequence"], name: "index_categories_on_sequence"
119120
t.index ["tag_set_id"], name: "index_categories_on_tag_set_id"
120121
end

‎test/controllers/abilities_controller_test.rb‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,80 @@ class AbilitiesControllerTest < ActionController::TestCase
6565
assert_not_nil assigns(:user)
6666
assert_not_nil assigns(:your_ability)
6767
end
68+
69+
test ':update should require authentication' do
70+
ability = abilities(:everyone)
71+
try_update_ability(ability, description: 'anonymous')
72+
assert_redirected_to_sign_in
73+
end
74+
75+
test ':update should correctly update abilities' do
76+
ability = abilities(:everyone)
77+
78+
users.each do |user|
79+
description = "#{user.name}'s edit"
80+
81+
sign_in user
82+
try_update_ability(ability, description: description)
83+
84+
if user.deleted? || user.community_user.deleted?
85+
assert_redirected_to_sign_in
86+
elsif user.can_edit_abilities?
87+
assert_redirected_to ability_url(id: ability.internal_id)
88+
assert_equal assigns(:ability).description, description
89+
else
90+
assert_response(:not_found)
91+
end
92+
end
93+
end
94+
95+
test ':update should prevent invalid updates' do
96+
ability = abilities(:everyone)
97+
old_name = ability.name
98+
user = users(:global_admin)
99+
100+
sign_in user
101+
102+
[false, true].each do |network_push|
103+
try_update_ability(ability, description: 'valid',
104+
name: '',
105+
network_push: network_push)
106+
assert_response(:bad_request)
107+
108+
ability.reload
109+
assert_equal ability.name, old_name
110+
end
111+
end
112+
113+
test ':update should allow global mods & admins to network push' do
114+
ability = abilities(:everyone)
115+
116+
users.select { |u| u.can_push_to_network?(ability) }.each do |user|
117+
description = "#{user.name}'s edit"
118+
119+
sign_in user
120+
try_update_ability(ability, description: description, network_push: true)
121+
assert_redirected_to ability_url(id: ability.internal_id)
122+
123+
network_abilities = Ability.unscoped.where(internal_id: ability.internal_id)
124+
assert network_abilities.any?
125+
126+
network_abilities.each do |network_ability|
127+
assert_equal network_ability.description, description
128+
end
129+
end
130+
end
131+
132+
private
133+
134+
# @param ability [Ability] ability to update
135+
def try_update_ability(ability, **opts)
136+
network_push = opts.delete(:network_push) || false
137+
138+
patch :update, params: {
139+
ability: {}.merge(opts),
140+
id: ability.internal_id,
141+
network_push: network_push
142+
}
143+
end
68144
end

0 commit comments

Comments
 (0)