forked from fixin.me/fixin.me
Compare commits
1 Commits
fix-sqlite
...
pr70-sole-
| Author | SHA1 | Date | |
|---|---|---|---|
| f626a814a8 |
@@ -1,6 +1,10 @@
|
|||||||
class User::ProfilesController < Devise::RegistrationsController
|
class User::ProfilesController < Devise::RegistrationsController
|
||||||
def destroy
|
def destroy
|
||||||
# TODO: Disallow/disable deletion for last admin account; update :edit view
|
if current_user.sole_admin?
|
||||||
|
redirect_back fallback_location: edit_user_registration_path,
|
||||||
|
alert: t(".sole_admin")
|
||||||
|
return
|
||||||
|
end
|
||||||
super
|
super
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
@@ -16,7 +16,7 @@ class Quantity < ApplicationRecord
|
|||||||
end
|
end
|
||||||
validates :name, presence: true, uniqueness: {scope: [:user_id, :parent_id]},
|
validates :name, presence: true, uniqueness: {scope: [:user_id, :parent_id]},
|
||||||
length: {maximum: type_for_attribute(:name).limit}
|
length: {maximum: type_for_attribute(:name).limit}
|
||||||
validates :description, length: {maximum: type_for_attribute(:description).limit} if type_for_attribute(:description).limit
|
validates :description, length: {maximum: type_for_attribute(:description).limit}
|
||||||
|
|
||||||
# Update :depths of progenies after parent change
|
# Update :depths of progenies after parent change
|
||||||
before_save if: :parent_changed? do
|
before_save if: :parent_changed? do
|
||||||
@@ -61,22 +61,18 @@ class Quantity < ApplicationRecord
|
|||||||
|
|
||||||
# Return: ordered [sub]hierarchy
|
# Return: ordered [sub]hierarchy
|
||||||
scope :ordered, ->(root: nil, include_root: true) {
|
scope :ordered, ->(root: nil, include_root: true) {
|
||||||
q_ordered = Arel::Table.new('q_ordered')
|
numbered = Arel::Table.new('numbered')
|
||||||
cast_type = connection.adapter_name == 'Mysql2' ? 'BINARY' : 'BLOB'
|
|
||||||
|
|
||||||
self.model.with(numbered: numbered(:parent_id, :name)).with_recursive(q_ordered: [
|
self.model.with(numbered: numbered(:parent_id, :name)).with_recursive(arel_table.name => [
|
||||||
Quantity.from(Arel::Table.new('numbered').as(arel_table.name))
|
numbered.project(
|
||||||
.select(
|
numbered[Arel.star],
|
||||||
arel_table[Arel.star],
|
numbered.cast(numbered[:child_number], 'BINARY').as('path')
|
||||||
arel_table.cast(arel_table[:child_number], cast_type).as('path')
|
).where(numbered[root && include_root ? :id : :parent_id].eq(root)),
|
||||||
).where(arel_table[root && include_root ? :id : :parent_id].eq(root)),
|
numbered.project(
|
||||||
Quantity.from(Arel::Table.new('numbered').as(arel_table.name))
|
numbered[Arel.star],
|
||||||
.joins("INNER JOIN q_ordered ON quantities.parent_id = q_ordered.id")
|
arel_table[:path].concat(numbered[:child_number])
|
||||||
.select(
|
).join(arel_table).on(numbered[:parent_id].eq(arel_table[:id]))
|
||||||
arel_table[Arel.star],
|
]).order(arel_table[:path])
|
||||||
q_ordered[:path].concat(arel_table[:child_number])
|
|
||||||
)
|
|
||||||
]).from(q_ordered.as(arel_table.name)).order(arel_table[:path])
|
|
||||||
}
|
}
|
||||||
|
|
||||||
# TODO: extract named functions to custom Arel extension
|
# TODO: extract named functions to custom Arel extension
|
||||||
@@ -84,24 +80,20 @@ class Quantity < ApplicationRecord
|
|||||||
# be merged with :ordered
|
# be merged with :ordered
|
||||||
# https://gist.github.com/ProGM/c6df08da14708dcc28b5ca325df37ceb#extending-arel
|
# https://gist.github.com/ProGM/c6df08da14708dcc28b5ca325df37ceb#extending-arel
|
||||||
scope :numbered, ->(parent_column, order_column) {
|
scope :numbered, ->(parent_column, order_column) {
|
||||||
row_number = Arel::Nodes::NamedFunction.new('ROW_NUMBER', [])
|
select(
|
||||||
.over(Arel::Nodes::Window.new.partition(parent_column).order(order_column))
|
arel_table[Arel.star],
|
||||||
width = Arel::SelectManager.new.project(
|
Arel::Nodes::NamedFunction.new(
|
||||||
|
'LPAD',
|
||||||
|
[
|
||||||
|
Arel::Nodes::NamedFunction.new('ROW_NUMBER', [])
|
||||||
|
.over(Arel::Nodes::Window.new.partition(parent_column).order(order_column)),
|
||||||
|
Arel::SelectManager.new.project(
|
||||||
Arel::Nodes::NamedFunction.new('LENGTH', [Arel.star.count])
|
Arel::Nodes::NamedFunction.new('LENGTH', [Arel.star.count])
|
||||||
).from(arel_table)
|
).from(arel_table),
|
||||||
|
Arel::Nodes.build_quoted('0')
|
||||||
pad_expr = if connection.adapter_name == 'Mysql2'
|
],
|
||||||
Arel::Nodes::NamedFunction.new('LPAD', [row_number, width, Arel::Nodes.build_quoted('0')])
|
).as('child_number')
|
||||||
else
|
|
||||||
# SQLite: printf('%0' || width || 'd', row_number)
|
|
||||||
fmt = Arel::Nodes::InfixOperation.new('||',
|
|
||||||
Arel::Nodes::InfixOperation.new('||', Arel::Nodes.build_quoted('%0'), width),
|
|
||||||
Arel::Nodes.build_quoted('d')
|
|
||||||
)
|
)
|
||||||
Arel::Nodes::NamedFunction.new('printf', [fmt, row_number])
|
|
||||||
end
|
|
||||||
|
|
||||||
select(arel_table[Arel.star], pad_expr.as('child_number'))
|
|
||||||
}
|
}
|
||||||
|
|
||||||
def to_s
|
def to_s
|
||||||
@@ -110,7 +102,7 @@ class Quantity < ApplicationRecord
|
|||||||
|
|
||||||
def to_s_with_depth
|
def to_s_with_depth
|
||||||
# em space, U+2003
|
# em space, U+2003
|
||||||
' ' * depth + name
|
' ' * depth + name
|
||||||
end
|
end
|
||||||
|
|
||||||
def destroyable?
|
def destroyable?
|
||||||
@@ -123,18 +115,16 @@ class Quantity < ApplicationRecord
|
|||||||
|
|
||||||
# Common ancestors, assuming node is a descendant of itself
|
# Common ancestors, assuming node is a descendant of itself
|
||||||
scope :common_ancestors, ->(of) {
|
scope :common_ancestors, ->(of) {
|
||||||
q_common = Arel::Table.new('q_common')
|
selected = Arel::Table.new('selected')
|
||||||
|
|
||||||
# Take unique IDs, so self can be called with parent nodes of collection to
|
# Take unique IDs, so self can be called with parent nodes of collection to
|
||||||
# get common ancestors of collection _excluding_ nodes in collection.
|
# get common ancestors of collection _excluding_ nodes in collection.
|
||||||
uniq_of = of.uniq
|
uniq_of = of.uniq
|
||||||
model.with(selected: self).with_recursive(q_common: [
|
model.with(selected: self).with_recursive(arel_table.name => [
|
||||||
Quantity.from(Arel::Table.new('selected').as(arel_table.name))
|
selected.project(selected[Arel.star]).where(selected[:id].in(uniq_of)),
|
||||||
.where(arel_table[:id].in(uniq_of)),
|
selected.project(selected[Arel.star])
|
||||||
Quantity.from(Arel::Table.new('selected').as(arel_table.name))
|
.join(arel_table).on(selected[:id].eq(arel_table[:parent_id]))
|
||||||
.joins("INNER JOIN q_common ON selected.id = q_common.parent_id")
|
|
||||||
]).select(arel_table[Arel.star])
|
]).select(arel_table[Arel.star])
|
||||||
.from(q_common.as(arel_table.name))
|
|
||||||
.group(column_names)
|
.group(column_names)
|
||||||
.having(arel_table[:id].count.eq(uniq_of.size))
|
.having(arel_table[:id].count.eq(uniq_of.size))
|
||||||
.order(arel_table[:depth].desc)
|
.order(arel_table[:depth].desc)
|
||||||
@@ -142,9 +132,13 @@ class Quantity < ApplicationRecord
|
|||||||
|
|
||||||
# Return: successive record in order of appearance; used for partial view reload
|
# Return: successive record in order of appearance; used for partial view reload
|
||||||
def successive
|
def successive
|
||||||
ordered = user.quantities.ordered.to_a
|
quantities = Quantity.arel_table
|
||||||
idx = ordered.index { |q| q.id == id }
|
Quantity.with(
|
||||||
ordered[idx + 1] if idx
|
quantities: user.quantities.ordered.select(
|
||||||
|
quantities[Arel.star],
|
||||||
|
Arel::Nodes::NamedFunction.new('LAG', [quantities[:id]]).over.as('lag_id')
|
||||||
|
)
|
||||||
|
).where(quantities[:lag_id].eq(id)).first
|
||||||
end
|
end
|
||||||
|
|
||||||
def with_progenies
|
def with_progenies
|
||||||
@@ -157,14 +151,13 @@ class Quantity < ApplicationRecord
|
|||||||
|
|
||||||
# Return: record with ID `of` with its ancestors, sorted by :depth
|
# Return: record with ID `of` with its ancestors, sorted by :depth
|
||||||
scope :with_ancestors, ->(of) {
|
scope :with_ancestors, ->(of) {
|
||||||
q_ancestors = Arel::Table.new('q_ancestors')
|
selected = Arel::Table.new('selected')
|
||||||
|
|
||||||
model.with(selected: self).with_recursive(q_ancestors: [
|
model.with(selected: self).with_recursive(arel_table.name => [
|
||||||
Quantity.from(Arel::Table.new('selected').as(arel_table.name))
|
selected.project(selected[Arel.star]).where(selected[:id].eq(of)),
|
||||||
.where(arel_table[:id].eq(of)),
|
selected.project(selected[Arel.star])
|
||||||
Quantity.from(Arel::Table.new('selected').as(arel_table.name))
|
.join(arel_table).on(selected[:id].eq(arel_table[:parent_id]))
|
||||||
.joins("INNER JOIN q_ancestors ON selected.id = q_ancestors.parent_id")
|
])
|
||||||
]).from(q_ancestors.as(arel_table.name))
|
|
||||||
}
|
}
|
||||||
|
|
||||||
# Return: ancestors of (possibly destroyed) self
|
# Return: ancestors of (possibly destroyed) self
|
||||||
|
|||||||
@@ -13,7 +13,7 @@ class Unit < ApplicationRecord
|
|||||||
end
|
end
|
||||||
validates :symbol, presence: true, uniqueness: {scope: :user_id},
|
validates :symbol, presence: true, uniqueness: {scope: :user_id},
|
||||||
length: {maximum: type_for_attribute(:symbol).limit}
|
length: {maximum: type_for_attribute(:symbol).limit}
|
||||||
validates :description, length: {maximum: type_for_attribute(:description).limit} if type_for_attribute(:description).limit
|
validates :description, length: {maximum: type_for_attribute(:description).limit}
|
||||||
validates :multiplier, numericality: {equal_to: 1}, unless: :base
|
validates :multiplier, numericality: {equal_to: 1}, unless: :base
|
||||||
validates :multiplier, numericality: {greater_than: 0, precision: true, scale: true}, if: :base
|
validates :multiplier, numericality: {greater_than: 0, precision: true, scale: true}, if: :base
|
||||||
|
|
||||||
@@ -22,23 +22,14 @@ class Unit < ApplicationRecord
|
|||||||
actionable_units = Arel::Table.new('actionable_units')
|
actionable_units = Arel::Table.new('actionable_units')
|
||||||
units = actionable_units.alias('units')
|
units = actionable_units.alias('units')
|
||||||
bases_units = arel_table.alias('bases_units')
|
bases_units = arel_table.alias('bases_units')
|
||||||
# Use 'all_units' alias for correlated subqueries to reference the all_units CTE.
|
other_units = arel_table.alias('other_units')
|
||||||
# Cannot use arel_table.alias here because the CTE is named 'all_units', not 'units'.
|
other_bases_units = arel_table.alias('other_bases_units')
|
||||||
other_units = Arel::Table.new('all_units').alias('other_units')
|
|
||||||
other_bases_units = Arel::Table.new('all_units').alias('other_bases_units')
|
|
||||||
sub_units = arel_table.alias('sub_units')
|
sub_units = arel_table.alias('sub_units')
|
||||||
|
|
||||||
Unit.with_recursive(
|
# TODO: move inner 'with' CTE to outer 'with recursive' - it can have multiple
|
||||||
# Renamed from 'units' to 'all_units' to avoid SQLite circular reference:
|
# CTEs, even non recursive ones.
|
||||||
# SQLite treats any CTE in WITH RECURSIVE that references a table with the same
|
Unit.with_recursive(actionable_units: [
|
||||||
# name as the CTE itself as a circular reference, even for non-recursive CTEs.
|
Unit.with(units: self.or(Unit.defaults)).left_joins(:base)
|
||||||
all_units: self.or(Unit.defaults),
|
|
||||||
actionable_units: [
|
|
||||||
# Read from all_units CTE (user+defaults) aliased as the units table name.
|
|
||||||
# Using AR::Relation (not Arel::SelectManager) to avoid extra parentheses
|
|
||||||
# around the UNION part (visit_Arel_SelectManager always wraps in parens).
|
|
||||||
Unit.from(Arel::Table.new('all_units').as(arel_table.name))
|
|
||||||
.joins("LEFT OUTER JOIN all_units bases_units ON bases_units.id = units.base_id")
|
|
||||||
.where.not(
|
.where.not(
|
||||||
# Exclude Units that are/have default counterpart
|
# Exclude Units that are/have default counterpart
|
||||||
Arel::SelectManager.new.project(1).from(other_units)
|
Arel::SelectManager.new.project(1).from(other_units)
|
||||||
@@ -74,20 +65,16 @@ class Unit < ApplicationRecord
|
|||||||
),
|
),
|
||||||
# Fill base Units to display proper hierarchy. Duplicates will be removed
|
# Fill base Units to display proper hierarchy. Duplicates will be removed
|
||||||
# by final group() - can't be deduplicated with UNION due to 'portable' field.
|
# by final group() - can't be deduplicated with UNION due to 'portable' field.
|
||||||
# Using AR::Relation instead of Arel::SelectManager to avoid extra parentheses
|
arel_table.join(actionable_units).on(actionable_units[:base_id].eq(arel_table[:id]))
|
||||||
# around the UNION part (visit_Arel_SelectManager always wraps in parens).
|
.project(arel_table[Arel.star], Arel::Nodes.build_quoted(nil).as('portable'))
|
||||||
Unit.from(Arel::Table.new('all_units').as(arel_table.name))
|
]).select(units: [:base_id, :symbol])
|
||||||
.joins("INNER JOIN actionable_units ON actionable_units.base_id = units.id")
|
|
||||||
.select(arel_table[Arel.star], Arel::Nodes.build_quoted(nil).as('portable'))
|
|
||||||
]
|
|
||||||
).select(units: [:base_id, :symbol])
|
|
||||||
.select(
|
.select(
|
||||||
units[:id].minimum.as('id'), # can be ANY_VALUE()
|
units[:id].minimum.as('id'), # can be ANY_VALUE()
|
||||||
units[:user_id].minimum.as('user_id'), # prefer non-default
|
units[:user_id].minimum.as('user_id'), # prefer non-default
|
||||||
Arel::Nodes.build_quoted(1).as('multiplier'), # disregard multiplier when sorting
|
Arel::Nodes.build_quoted(1).as('multiplier'), # disregard multiplier when sorting
|
||||||
units[:portable].minimum.as('portable')
|
units[:portable].minimum.as('portable')
|
||||||
)
|
)
|
||||||
.from(units).group(units[:base_id], units[:symbol])
|
.from(units).group(:base_id, :symbol)
|
||||||
}
|
}
|
||||||
scope :ordered, ->{
|
scope :ordered, ->{
|
||||||
left_outer_joins(:base).order(ordering)
|
left_outer_joins(:base).order(ordering)
|
||||||
@@ -97,7 +84,7 @@ class Unit < ApplicationRecord
|
|||||||
[arel_table.coalesce(Arel::Table.new(:bases_units)[:symbol], arel_table[:symbol]),
|
[arel_table.coalesce(Arel::Table.new(:bases_units)[:symbol], arel_table[:symbol]),
|
||||||
arel_table[:base_id].not_eq(nil),
|
arel_table[:base_id].not_eq(nil),
|
||||||
:multiplier,
|
:multiplier,
|
||||||
arel_table[:symbol]]
|
:symbol]
|
||||||
end
|
end
|
||||||
|
|
||||||
before_destroy do
|
before_destroy do
|
||||||
|
|||||||
@@ -29,4 +29,11 @@ class User < ApplicationRecord
|
|||||||
def at_least(status)
|
def at_least(status)
|
||||||
User.statuses[self.status] >= User.statuses[status]
|
User.statuses[self.status] >= User.statuses[status]
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Returns true when this user is the only admin account in the system.
|
||||||
|
# Used to block actions that would leave the application without an admin
|
||||||
|
# (account deletion, status demotion).
|
||||||
|
def sole_admin?
|
||||||
|
admin? && !User.admin.where.not(id: id).exists?
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -4,9 +4,8 @@
|
|||||||
<% end %>
|
<% end %>
|
||||||
|
|
||||||
<div class="rightside-area buttongrid">
|
<div class="rightside-area buttongrid">
|
||||||
<%#= TODO: Disallow/disable deletion for last admin account, image_button_to_if %>
|
<%= image_button_to_if !current_user.sole_admin?, t('.delete'), 'account-remove-outline',
|
||||||
<%= image_button_to t('.delete'), 'account-remove-outline', user_registration_path,
|
user_registration_path, form_class: 'tools-area', method: :delete, data: {turbo: false},
|
||||||
form_class: 'tools-area', method: :delete, data: {turbo: false},
|
|
||||||
onclick: {confirm: t('.confirm_delete')} %>
|
onclick: {confirm: t('.confirm_delete')} %>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
|||||||
@@ -162,6 +162,9 @@ en:
|
|||||||
New password:
|
New password:
|
||||||
<br><em>leave blank to keep unchanged</em>
|
<br><em>leave blank to keep unchanged</em>
|
||||||
%{password_length_hint_html}
|
%{password_length_hint_html}
|
||||||
|
registrations:
|
||||||
|
destroy:
|
||||||
|
sole_admin: You cannot delete the only admin account.
|
||||||
actions: Actions
|
actions: Actions
|
||||||
add: Add
|
add: Add
|
||||||
apply: Apply
|
apply: Apply
|
||||||
|
|||||||
18
test/controllers/registrations_controller_test.rb
Normal file
18
test/controllers/registrations_controller_test.rb
Normal file
@@ -0,0 +1,18 @@
|
|||||||
|
require "test_helper"
|
||||||
|
|
||||||
|
class RegistrationsControllerTest < ActionDispatch::IntegrationTest
|
||||||
|
test "sole admin cannot delete account" do
|
||||||
|
sign_in users(:admin)
|
||||||
|
delete user_registration_path
|
||||||
|
assert_redirected_to edit_user_registration_path
|
||||||
|
assert_equal t("registrations.destroy.sole_admin"), flash[:alert]
|
||||||
|
assert User.exists?(users(:admin).id)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "non-admin can delete account" do
|
||||||
|
sign_in users(:alice)
|
||||||
|
assert_difference ->{ User.count }, -1 do
|
||||||
|
delete user_registration_path
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
@@ -182,8 +182,8 @@ class UsersTest < ApplicationSystemTestCase
|
|||||||
assert_title 'Access is forbidden to this page (403)'
|
assert_title 'Access is forbidden to this page (403)'
|
||||||
end
|
end
|
||||||
|
|
||||||
test 'delete profile' do
|
test "delete profile" do
|
||||||
user = sign_in
|
user = sign_in user: users.reject(&:admin?).select(&:confirmed?).sample
|
||||||
# TODO: remove condition after root_url changed to different path than
|
# TODO: remove condition after root_url changed to different path than
|
||||||
# profile in routes.rb
|
# profile in routes.rb
|
||||||
unless has_current_path?(edit_user_registration_path)
|
unless has_current_path?(edit_user_registration_path)
|
||||||
@@ -196,7 +196,15 @@ class UsersTest < ApplicationSystemTestCase
|
|||||||
assert_text t("devise.registrations.destroyed")
|
assert_text t("devise.registrations.destroyed")
|
||||||
end
|
end
|
||||||
|
|
||||||
test 'index forbidden for non admin' do
|
test "sole admin cannot delete profile" do
|
||||||
|
sign_in user: users(:admin)
|
||||||
|
unless has_current_path?(edit_user_registration_path)
|
||||||
|
first(:link_or_button, users(:admin).email).click
|
||||||
|
end
|
||||||
|
assert find(:button, t("users.registrations.edit.delete"))[:disabled]
|
||||||
|
end
|
||||||
|
|
||||||
|
test "index forbidden for non admin" do
|
||||||
sign_in user: users.reject(&:admin?).select(&:confirmed?).sample
|
sign_in user: users.reject(&:admin?).select(&:confirmed?).sample
|
||||||
visit users_path
|
visit users_path
|
||||||
assert_title "Access is forbidden to this page (403)"
|
assert_title "Access is forbidden to this page (403)"
|
||||||
|
|||||||
Reference in New Issue
Block a user