Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log actions on static pages #2754

Merged
merged 14 commits into from Feb 19, 2018
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -15,12 +15,15 @@
- **decidim-participatory_processes**: Ensure only active processes are shown in the highlighted processes section in the homepage[\#2682](https://github.com/decidim/decidim/pull/2682)
- **decidim-core**: Add collections to group attachments [\#2394](https://github.com/decidim/decidim/pull/2394).
- **decidim-admin**: Adds a log of all admin actions, only visible by organization admins [\#2604](https://github.com/decidim/decidim/pull/2604)
- **decidim-core**: Make static pages traceable [\#2754](https://github.com/decidim/decidim/pull/2754)
- **decidim-admin**: Log all actions on static pages [\#2754](https://github.com/decidim/decidim/pull/2754)

**Changed**:

- **decidim-core**: General improvements on documentation [\#2656](https://github.com/decidim/decidim/pull/2656).
- **decidim-core**: `FeatureReferenceHelper#feature_reference` has been moved to `ResourceReferenceHelper#resource_reference` to clarify its use. [\#2557](https://github.com/decidim/decidim/pull/2557)
- **decidim-core**: `Decidim.resource_reference_generator` has been moved to `Decidim.reference_generator` to clarify its use. [\#2557](https://github.com/decidim/decidim/pull/2557)
- **decidim-system**: Default pages content are now wrapped in `<p>` HTML tags [\#2754](https://github.com/decidim/decidim/pull/2754)

**Fixed**:

Expand Down
Expand Up @@ -29,7 +29,9 @@ def call
attr_reader :form

def create_page
StaticPage.create!(
Decidim.traceability.create!(
StaticPage,
form.current_user,
title: form.title,
slug: form.slug,
content: form.content,
Expand Down
40 changes: 40 additions & 0 deletions decidim-admin/app/commands/decidim/admin/destroy_static_page.rb
@@ -0,0 +1,40 @@
# frozen_string_literal: true

module Decidim
module Admin
# This command deals with destroying a StaticPage from the admin panel.
class DestroyStaticPage < Rectify::Command
# Public: Initializes the command.
#
# page - The StaticPage to be destroyed.
def initialize(page, current_user)
@page = page
@current_user = current_user
end

# Public: Executes the command.
#
# Broadcasts :ok if it got destroyed
def call
destroy_page
broadcast(:ok)
end

private

attr_reader :page, :current_user

def destroy_page
transaction do
page.destroy!

Decidim::ActionLogger.log(
"delete",
current_user,
page
)
end
end
end
end
end
Expand Up @@ -31,7 +31,11 @@ def call
attr_reader :form

def update_page
@page.update_attributes!(attributes)
Decidim.traceability.update!(
@page,
form.current_user,
attributes
)
end

def attributes
Expand Down
Expand Up @@ -63,11 +63,13 @@ def show

def destroy
authorize! :destroy, page
page.destroy!

flash[:notice] = I18n.t("static_pages.destroy.success", scope: "decidim.admin")

redirect_to static_pages_path
DestroyStaticPage.call(page, current_user) do
on(:ok) do
flash[:notice] = I18n.t("static_pages.destroy.success", scope: "decidim.admin")
redirect_to static_pages_path
end
end
end

private
Expand Down
12 changes: 11 additions & 1 deletion decidim-admin/spec/commands/create_static_page_spec.rb
Expand Up @@ -6,7 +6,12 @@ module Decidim::Admin
describe CreateStaticPage do
describe "call" do
let(:organization) { create(:organization) }
let(:form) { StaticPageForm.from_model(build(:static_page)).with_context(current_organization: organization) }
let(:user) { create :user, :admin, :confirmed, organization: organization }
let(:form) do
StaticPageForm
.from_model(build(:static_page))
.with_context(current_user: user, current_organization: organization)
end
let(:command) { described_class.new(form) }

describe "when the form is not valid" do
Expand All @@ -30,6 +35,11 @@ module Decidim::Admin
expect { command.call }.to broadcast(:ok)
end

it "uses traceability to create the page" do
expect(Decidim.traceability).to receive(:create!)
command.call
end

it "creates a page in the organization" do
expect do
command.call
Expand Down
27 changes: 27 additions & 0 deletions decidim-admin/spec/commands/destroy_static_page_spec.rb
@@ -0,0 +1,27 @@
# frozen_string_literal: true

require "spec_helper"

module Decidim::Admin
describe DestroyStaticPage do
subject { described_class.new(page, user) }

let!(:page) { create(:static_page) }
let!(:user) { create(:user, organization: page.organization) }

context "when everything is ok" do
it "destroys the page" do
subject.call
expect(Decidim::StaticPage.where(id: page.id)).not_to exist
end

it "logs the action" do
expect(Decidim::ActionLogger)
.to receive(:log)
.with("delete", user, page)

subject.call
end
end
end
end
9 changes: 8 additions & 1 deletion decidim-admin/spec/commands/update_static_page_spec.rb
Expand Up @@ -7,11 +7,13 @@ module Decidim::Admin
describe "call" do
let(:organization) { create(:organization) }
let(:page) { create(:static_page, organization: organization) }
let(:user) { create :user, :admin, :confirmed, organization: organization }
let(:form) do
StaticPageForm.from_params(
static_page: page.attributes.merge(slug: "new-slug")
).with_context(
current_organization: page.organization
current_organization: page.organization,
current_user: user
)
end
let(:command) { described_class.new(page, form) }
Expand All @@ -38,6 +40,11 @@ module Decidim::Admin
expect { command.call }.to broadcast(:ok)
end

it "traces the update" do
expect(Decidim.traceability).to receive(:update!)
command.call
end

it "updates the page in the organization" do
command.call
page.reload
Expand Down
2 changes: 2 additions & 0 deletions decidim-core/app/models/decidim/static_page.rb
Expand Up @@ -8,6 +8,8 @@ module Decidim
# Pages with a default slug cannot be destroyed and its slug cannot be
# modified.
class StaticPage < ApplicationRecord
include Decidim::Traceable

belongs_to :organization, foreign_key: "decidim_organization_id", class_name: "Decidim::Organization", inverse_of: :static_pages

validates :slug, presence: true, uniqueness: { scope: :organization }
Expand Down
@@ -0,0 +1,35 @@
# frozen_string_literal: true

module Decidim
module AdminLog
# This class holds the logic to present a `Decidim::StaticPage`
# for the `AdminLog` log.
#
# Usage should be automatic and you shouldn't need to call this class
# directly, but here's an example:
#
# action_log = Decidim::ActionLog.last
# view_helpers # => this comes from the views
# StaticPagePresenter.new(action_log, view_helpers).present
class StaticPagePresenter < Decidim::Log::BasePresenter
private

def diff_fields_mapping
{
content: :i18n,
slug: :default,
title: :i18n
}
end

# Private: Caches the object that will be responsible of presenting the static page.
# Overwrites the method so that we can use a custom presenter to show the correct
# path for ther page.
#
# Returns an object that responds to `present`.
def resource_presenter
@resource_presenter ||= Decidim::AdminLog::StaticPageResourcePresenter.new(action_log.resource, h, action_log.extra["resource"])
end
end
end
end
@@ -0,0 +1,18 @@
# frozen_string_literal: true

module Decidim
module AdminLog
# This class extends the default resource presenter for logs, so that
# it can properly link to the static page.
class StaticPageResourcePresenter < Decidim::Log::ResourcePresenter
private

# Private: Finds the public link for the given static page..
#
# Returns an HTML-safe String.
def resource_path
@resource_path ||= h.decidim.page_path(resource)
end
end
end
end
35 changes: 29 additions & 6 deletions decidim-core/app/presenters/decidim/log/base_presenter.rb
Expand Up @@ -152,7 +152,8 @@ def present_diff
#
# Returns an HTML-safe String.
def present_action_log
h.content_tag(:li, id: h.dom_id(action_log), class: "logs__log", data: { toggler: ".logs__log--expanded" }) do
classes = ["logs__log"] + action_log_extra_classes.to_a
h.content_tag(:li, id: h.dom_id(action_log), class: classes.join(" "), data: { toggler: ".logs__log--expanded" }) do
h.concat(present_content)
h.concat(present_diff)
end
Expand All @@ -164,15 +165,28 @@ def present_action_log
# Returns a String.
def action_string
case action.to_s
when "create"
"decidim.log.base_presenter.create"
when "update"
"decidim.log.base_presenter.update"
when "create", "update", "delete"
generate_action_string(action)
else
"decidim.log.base_presenter.unknown_action"
generate_action_string(:unknown_action)
end
end

# Private: Generates the correct action string considering if
# the space is present or not.
#
# action - A String with the name of the action
#
# Returns a String.
def generate_action_string(action)
string = if action_log.participatory_space.present?
"#{action}_with_space"
else
action
end
"decidim.log.base_presenter.#{string}"
end

# Private: The params to be sent to the i18n string.
#
# Returns a Hash.
Expand Down Expand Up @@ -209,6 +223,15 @@ def diff_fields_mapping
# Returns a String.
def i18n_labels_scope; end

# Private: Holds a list of extra classes to apply to the action log
# HTML element.
#
# Returns an Array of Strings.
def action_log_extra_classes
return ["logs__log--deletion"] if action.to_s == "delete"
[]
end

# Private: Calculates the changeset to be rendered. Uses the values
# from the `diff_fields_mapping` method.
#
Expand Down
Expand Up @@ -75,6 +75,8 @@ def calculate_changeset(attribute, values, type)
#
# Returns an array of hashes.
def generate_i18n_changeset(attribute, values, type)
values.map! { |value| value.is_a?(String) ? JSON.parse(value) : value }

values.last.flat_map do |locale, _value|
previous_value = values.first.try(:[], locale)
new_value = values.last.try(:[], locale)
Expand Down Expand Up @@ -111,13 +113,17 @@ def generate_changeset(attribute, values, type, label = nil)
# Generates the label for the given attribute. If the `locale` is set,
# it appends the locale at the end: `AttributeName (LocaleName)`.
#
# attribute - A String representing the attribute name. It will retrive
# attribute - A Symbol representing the attribute name. It will retrive
# this key from the I18n scope set at `i18n_labels_scope`.
# locale - a String representing the name of the locale.
#
# Returns a String.
def generate_label(attribute, locale = nil)
label = I18n.t(attribute, scope: i18n_labels_scope)
label = if i18n_labels_scope
I18n.t(attribute, scope: i18n_labels_scope)
else
attribute.to_s.humanize
end
return label unless locale

locale_name = I18n.t("locale.name", locale: locale)
Expand Down
11 changes: 8 additions & 3 deletions decidim-core/config/locales/en.yml
Expand Up @@ -202,9 +202,14 @@ en:
remove_this_file: Remove this file
log:
base_presenter:
create: "%{user_name} created %{resource_name} in %{space_name}"
unknown_action: "%{user_name} performed some action on %{resource_name} in %{space_name}"
update: "%{user_name} updated %{resource_name} in %{space_name}"
create: "%{user_name} created %{resource_name}"
create_with_space: "%{user_name} created %{resource_name} in %{space_name}"
delete: "%{user_name} deleted %{resource_name}"
delete_with_space: "%{user_name} deleted %{resource_name} in %{space_name}"
unknown_action: "%{user_name} performed some action on %{resource_name}"
unknown_action_with_space: "%{user_name} performed some action on %{resource_name} in %{space_name}"
update: "%{user_name} updated %{resource_name}"
update_with_space: "%{user_name} updated %{resource_name} in %{space_name}"
value_types:
scope_presenter:
not_found: 'The scope was not found on the database (ID: %{id})'
Expand Down
2 changes: 2 additions & 0 deletions decidim-core/spec/models/decidim/static_page_spec.rb
Expand Up @@ -6,6 +6,8 @@ module Decidim
describe StaticPage do
let(:page) { build(:static_page) }

it { is_expected.to be_versioned }

describe "validations" do
let(:invalid_slug) { "#Invalid.Slug" }

Expand Down