Skip to content

Commit

Permalink
Fixes zammad#4381 - Ticket templates are missing active flag.
Browse files Browse the repository at this point in the history
  • Loading branch information
dvuckovic committed Dec 6, 2022
1 parent 08bfd43 commit 0f7d08e
Show file tree
Hide file tree
Showing 13 changed files with 216 additions and 12 deletions.
6 changes: 2 additions & 4 deletions app/assets/javascripts/app/models/template.coffee
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
class App.Template extends App.Model
@configure 'Template', 'name', 'options', 'user_id', 'updated_at'
@configure 'Template', 'name', 'options', 'user_id', 'updated_at', 'active'
@extend Spine.Model.Ajax
@url: @apiPath + '/templates'
@configure_attributes = [
{ name: 'name', display: __('Name'), tag: 'input', type: 'text', limit: 100, null: false },
{ name: 'options', display: __('Actions'), tag: 'ticket_perform_action', user_action: false, article_body_only: true, no_richtext_uploads: true, sender_type: true, skip_unknown_attributes: true, null: true },
{ name: 'updated_at', display: __('Updated'), tag: 'datetime', readonly: 1 },

# TODO: Extend model to support storing of the active flag.
# { name: 'active', display: __('Active'), tag: 'active', default: true },
{ name: 'active', display: __('Active'), tag: 'active', default: true },
]
@configure_delete = true
@configure_clone = true
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/templates_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class TemplatesController < ApplicationController
=end

def index
model_index_render(Template, params)
model_index_render(policy_scope(Template), params)
end

=begin
Expand All @@ -76,7 +76,7 @@ def index
=end

def show
model_show_render(Template, params)
model_show_render(policy_scope(Template), params)
end

=begin
Expand Down
2 changes: 2 additions & 0 deletions app/models/template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ class Template < ApplicationModel
include ChecksClientNotification
include Template::Assets

scope :active, -> { where(active: true) }

store :options
validates :name, presence: true

Expand Down
3 changes: 2 additions & 1 deletion app/policies/controllers/templates_controller_policy.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/

class Controllers::TemplatesControllerPolicy < Controllers::ApplicationControllerPolicy
default_permit!(['ticket.agent', 'admin.template'])
default_permit!('admin.template')
permit! %i[index show], to: ['admin.template', 'ticket.agent']
end
4 changes: 4 additions & 0 deletions app/policies/template_policy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/

class TemplatePolicy < ApplicationPolicy
end
17 changes: 17 additions & 0 deletions app/policies/template_policy/scope.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/

class TemplatePolicy < ApplicationPolicy
class Scope < ApplicationPolicy::Scope

def resolve
if user.permissions?('admin.template')
scope.all
elsif user.permissions?('ticket.agent')
scope.active
else
scope.none
end
end

end
end
5 changes: 3 additions & 2 deletions db/migrate/20120101000010_create_ticket.rb
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,9 @@ def up
create_table :templates do |t|
t.column :name, :string, limit: 250, null: false
t.column :options, :text, limit: 10.megabytes + 1, null: false
t.column :updated_by_id, :integer, null: false
t.column :created_by_id, :integer, null: false
t.column :active, :boolean, null: false, default: true
t.column :updated_by_id, :integer, null: false
t.column :created_by_id, :integer, null: false
t.timestamps limit: 3, null: false
end
add_index :templates, [:name]
Expand Down
14 changes: 14 additions & 0 deletions db/migrate/20221205151816_issue_4381_templates_active_field.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/

class Issue4381TemplatesActiveField < ActiveRecord::Migration[6.1]
def change
# return if it's a new setup
return if !Setting.exists?(name: 'system_init_done')

change_table :templates do |t|
t.boolean :active, default: true, null: false
end

Template.reset_column_information
end
end
1 change: 1 addition & 0 deletions spec/factories/template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
options { { 'ticket.title': { value: 'Some dummy title' } } }
updated_by_id { 1 }
created_by_id { 1 }
active { true }

transient do
title { 'Title dummy.' }
Expand Down
51 changes: 51 additions & 0 deletions spec/policies/template_policy/scope_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/

require 'rails_helper'

RSpec.describe TemplatePolicy::Scope do
subject(:scope) { described_class.new(user, original_collection) }

let(:original_collection) { Template }

let(:active_template) { create(:template, :dummy_data, active: true) }
let(:inactive_template) { create(:template, :dummy_data, active: false) }

before do
Template.destroy_all
active_template && inactive_template
end

describe '#resolve' do
context 'without user' do
let(:user) { nil }

it 'throws exception' do
expect { scope.resolve }.to raise_error %r{Authentication required}
end
end

context 'with customer' do
let(:user) { create(:customer) }

it 'returns empty' do
expect(scope.resolve).to be_empty
end
end

context 'with agent' do
let(:user) { create(:agent) }

it 'returns active template only' do
expect(scope.resolve).to match_array [active_template]
end
end

context 'with admin' do
let(:user) { create(:admin) }

it 'returns all templates' do
expect(scope.resolve).to match_array [active_template, inactive_template]
end
end
end
end
73 changes: 71 additions & 2 deletions spec/requests/template_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,24 @@

RSpec.describe Template, type: :request do
let(:agent) { create(:agent) }
let(:admin) { create(:user, roles: Role.where(name: 'Admin')) }
let(:customer) { create(:customer) }

describe 'request handling', authenticated_as: :agent do
describe 'request handling', authenticated_as: :admin do
before do
allow(ActiveSupport::Deprecation).to receive(:warn)
end

context 'when listing templates' do
let!(:templates) { create_list(:template, 10) }
let!(:templates) do
create_list(:template, 10).tap do |templates|
templates.each do |template|

# Make all templates with even IDs inactive (total of 5).
template.update!(active: false) if template.id.even?
end
end
end

it 'returns all' do
get '/api/v1/templates.json'
Expand All @@ -27,6 +36,14 @@
expect(json_response[index]['options']).to eq(template.options)
end
end

context 'with agent permissions', authenticated_as: :agent do
it 'returns active templates only' do
get '/api/v1/templates.json'

expect(json_response.length).to eq(5)
end
end
end

context 'when showing templates' do
Expand All @@ -43,6 +60,34 @@

expect(json_response['options']).to eq(template.options)
end

context 'with inactive template' do
let!(:inactive_template) { create(:template, active: false) }

it 'returns ok' do
get "/api/v1/templates/#{inactive_template.id}.json"

expect(response).to have_http_status(:ok)
end
end

context 'with agent permissions', authenticated_as: :agent do
it 'returns ok' do
get "/api/v1/templates/#{template.id}.json"

expect(response).to have_http_status(:ok)
end

context 'with inactive template' do
let!(:inactive_template) { create(:template, active: false) }

it 'request is not found' do
get "/api/v1/templates/#{inactive_template.id}.json"

expect(response).to have_http_status(:not_found)
end
end
end
end

context 'when creating template' do
Expand All @@ -65,6 +110,14 @@

expect(ActiveSupport::Deprecation).to have_received(:warn)
end

context 'with agent permissions', authenticated_as: :agent do
it 'request is forbidden' do
post '/api/v1/templates.json', params: { name: 'Foo', options: { 'ticket.title': { value: 'Bar' } } }

expect(response).to have_http_status(:forbidden)
end
end
end

context 'when updating template' do
Expand All @@ -89,6 +142,14 @@

expect(ActiveSupport::Deprecation).to have_received(:warn)
end

context 'with agent permissions', authenticated_as: :agent do
it 'request is forbidden' do
put "/api/v1/templates/#{template.id}.json", params: { options: { 'ticket.title': { value: 'Foo' } } }

expect(response).to have_http_status(:forbidden)
end
end
end

context 'when destroying template' do
Expand All @@ -99,6 +160,14 @@

expect(response).to have_http_status(:ok)
end

context 'with agent permissions', authenticated_as: :agent do
it 'request is forbidden' do
delete "/api/v1/templates/#{template.id}.json"

expect(response).to have_http_status(:forbidden)
end
end
end
end
end
35 changes: 34 additions & 1 deletion spec/system/manage/template_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@
click_button('Submit')
end
end

it 'with inactive state' do
in_modal do
fill_in('name', with: Faker::Name.unique.name)
find_field('active').select('inactive')
click_button('Submit')

await_empty_ajax_queue

expect(Template.last.active).to be(false)
end
end
end

context 'when editing an existing template' do
Expand Down Expand Up @@ -77,7 +89,7 @@
it 'ignores unknown or invalid attributes (#4316)' do
in_modal do
check_input_field_value('options::ticket.title::value', 'Test')
expect(find_all('select.form-control').length).to eq(1)
expect(find_all('select.form-control').length).to eq(2)
end
end
end
Expand Down Expand Up @@ -181,5 +193,26 @@
end
end
end

context 'with active field' do
let(:template) do
create(:template,
:dummy_data,
active: false)
end

it 'shows inactive selection' do
check_select_field_value('active', 'false')
end

it 'supports modifying active state' do
find_field('active').select('active')
click_button('Submit')

await_empty_ajax_queue

expect(template.reload.active).to be(true)
end
end
end
end
13 changes: 13 additions & 0 deletions spec/system/ticket/create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1422,4 +1422,17 @@ def authenticate
end
end
end

describe 'Ticket templates are missing active flag #4381' do
let!(:active_template) { create(:template, :dummy_data, active: true) }
let!(:inactive_template) { create(:template, :dummy_data, active: false) }

before do
visit 'ticket/create'
end

it 'filters active templates only' do
expect(find('#form-template select[name="id"]')).to have_selector('option', text: active_template.name).and(have_no_selector('option', text: inactive_template.name))
end
end
end

0 comments on commit 0f7d08e

Please sign in to comment.