Skip to content

Commit

Permalink
Merge pull request #8655 from alphagov/add-worldwideorg-role
Browse files Browse the repository at this point in the history
Add role association to Editionable Worldwide Organisations
  • Loading branch information
brucebolt authored Dec 22, 2023
2 parents 7ab9995 + f601c8b commit cd09bb6
Show file tree
Hide file tree
Showing 23 changed files with 241 additions and 3 deletions.
1 change: 1 addition & 0 deletions app/controllers/admin/editions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ def permitted_edition_attributes
lead_organisation_ids: [],
supporting_organisation_ids: [],
organisation_ids: [],
role_ids: [],
world_location_ids: [],
worldwide_organisation_ids: [],
related_policy_ids: [],
Expand Down
14 changes: 14 additions & 0 deletions app/helpers/admin/taggable_content_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,14 @@ def taggable_world_locations_container
end
end

# Returns an Array that represents the taggable roles. Each element of the
# array consists of two values: the role name and its ID
def taggable_roles_container
Rails.cache.fetch(taggable_roles_cache_digest, expires_in: 1.day) do
Role.order(:name).map { |w| [w.name, w.id] }
end
end

# Returns an Array that represents the taggable alternative format providers.
# Each element of the array consists of two values: the label (organisation
# and the email address if avaiable) and the ID of the organisation.
Expand Down Expand Up @@ -183,6 +191,12 @@ def taggable_world_locations_cache_digest
@taggable_world_locations_cache_digest ||= calculate_digest(WorldLocation.order(:id), "world-locations")
end

# Returns an MD5 digest representing the taggable roles. This will
# change if any world locations are added or updated.
def taggable_roles_cache_digest
@taggable_roles_cache_digest ||= calculate_digest(Role.order(:id), "roles")
end

# Returns an MD5 digest representing the taggable alternative format
# providers. This will change if any alternative format providers are
# changed.
Expand Down
4 changes: 4 additions & 0 deletions app/models/edition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,10 @@ def can_be_associated_with_topical_events?
false
end

def can_be_associated_with_roles?
false
end

def can_be_associated_with_role_appointments?
false
end
Expand Down
22 changes: 22 additions & 0 deletions app/models/edition/roles.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
module Edition::Roles
extend ActiveSupport::Concern

class Trait < Edition::Traits::Trait
def process_associations_before_save(edition)
@edition.edition_roles.each do |association|
edition.edition_roles.build(association.attributes.except("id", "edition_id"))
end
end
end

included do
has_many :edition_roles, foreign_key: :edition_id, inverse_of: :edition, dependent: :destroy, autosave: true
has_many :roles, through: :edition_roles

add_trait Trait
end

def can_be_associated_with_roles?
true
end
end
6 changes: 6 additions & 0 deletions app/models/edition_role.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class EditionRole < ApplicationRecord
belongs_to :edition
belongs_to :role, inverse_of: :edition_roles

validates :edition, :role, presence: true
end
1 change: 1 addition & 0 deletions app/models/editionable_worldwide_organisation.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class EditionableWorldwideOrganisation < Edition
include Edition::Organisations
include Edition::Roles
include Edition::WorldLocations

def display_type_key
Expand Down
5 changes: 5 additions & 0 deletions app/models/role.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ def self.columns
super.reject { |column| %w[name responsibilities].include?(column.name) }
end

has_many :edition_roles, inverse_of: :role
has_many :editions, through: :edition_roles

has_many :role_appointments, -> { order(started_at: :desc) }
has_many :people, through: :role_appointments

Expand Down Expand Up @@ -53,6 +56,8 @@ def self.columns
after_update :touch_role_appointments
after_save :republish_organisations_to_publishing_api, :republish_worldwide_organisations_to_publishing_api

accepts_nested_attributes_for :edition_roles

extend FriendlyId
friendly_id

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def content

def links
{
roles: item.roles.map(&:content_id),
sponsoring_organisations: item.organisations.map(&:content_id),
world_locations: item.world_locations.map(&:content_id),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
} do %>
<div class="govuk-!-margin-bottom-4">
<%= render "world_location_fields", form: form, edition: edition %>
<%= render "role_fields", form: form, edition: edition %>
<%= render "organisation_fields", form: form, edition: edition, required: true %>
</div>
<% end %>
Expand Down
9 changes: 9 additions & 0 deletions app/views/admin/editions/_conflict.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@
<% end %>
<% end %>

<% if edition.can_be_associated_with_roles? %>
<h2 class="govuk-heading-m">Roles</h2>
<% if edition.roles.any? %>
<%= render "admin/roles/list", roles: edition.roles %>
<% else %>
<p class="govuk-body">This document isn’t assigned to any roles.</p>
<% end %>
<% end %>

<% if edition.can_apply_to_subset_of_nations? %>
<h2 class="govuk-heading-m">Excluded nations</h2>
<% if edition.nation_inapplicabilities.any? %>
Expand Down
23 changes: 23 additions & 0 deletions app/views/admin/editions/_role_fields.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<% required = false unless defined?(required) %>

<% cache_if edition.role_ids.empty?, "#{taggable_roles_cache_digest}-design-system" do %>
<%= render "components/autocomplete", {
id: "edition_roles",
name: "edition[role_ids][]",
error_items: errors_for(edition.errors, :roles),
label: {
text: "Roles" + "#{' (required)' if required}",
heading_size: "m",
},
select: {
options: taggable_roles_container,
multiple: true,
selected: edition.role_ids,
},
data: {
module: "track-select-click",
track_category: "worldLocationSelection",
track_label: request.path,
},
} %>
<% end %>
7 changes: 7 additions & 0 deletions app/views/admin/roles/_list.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<ul class="roles">
<% roles.each do |role| %>
<%= content_tag_for(:li, roles) do %>
<%= link_to role.name, admin_role_path(anchor: "role_#{role.id}"), class: "location-link" %>
<% end %>
<% end %>
</ul>
5 changes: 5 additions & 0 deletions db/migrate/20231219154010_add_edition_roles.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddEditionRoles < ActiveRecord::Migration[7.0]
def change
create_join_table :edition, :roles, &:timestamps
end
end
9 changes: 8 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.0].define(version: 2023_12_12_165213) do
ActiveRecord::Schema[7.0].define(version: 2023_12_19_154010) do
create_table "assets", charset: "utf8mb3", force: :cascade do |t|
t.string "asset_manager_id", null: false
t.string "variant", null: false
Expand Down Expand Up @@ -302,6 +302,13 @@
t.index ["role_appointment_id"], name: "index_edition_role_appointments_on_role_appointment_id"
end

create_table "edition_roles", id: false, charset: "utf8mb3", force: :cascade do |t|
t.bigint "edition_id", null: false
t.bigint "role_id", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end

create_table "edition_statistical_data_sets", id: :integer, charset: "utf8mb3", force: :cascade do |t|
t.integer "edition_id"
t.integer "document_id"
Expand Down
6 changes: 6 additions & 0 deletions features/editionable-worldwide-organisations.feature
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,9 @@ Feature: Editionable worldwide organisations
When I draft a new worldwide organisation "Test Worldwide Organisation" assigned to world location "United Kingdom"
Then the worldwide organisation "Test Worldwide Organisation" should have been created
And I should see it has been assigned to the "United Kingdom" world location

Scenario Outline: Assigning a role to a worldwide organisation
Given a role "Prime Minister" exists
When I draft a new worldwide organisation "Test Worldwide Organisation" assigned to world location "United Kingdom"
And I edit the worldwide organisation "Test Worldwide Organisation" adding the role of "Prime Minister"
Then I should see the "Prime Minister" role has been assigned to the worldwide organisation "Test Worldwide Organisation"
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,18 @@
And(/^I should see it has been assigned to the "([^"]*)" world location$/) do |title|
expect(@worldwide_organisation.world_locations.first.name).to eq(title)
end

Given(/^a role "([^"]*)" exists$/) do |name|
create(:role, name:)
end

And(/^I edit the worldwide organisation "([^"]*)" adding the role of "([^"]*)"$/) do |title, role|
begin_editing_document(title)
select role, from: "Roles"
click_button "Save and go to document summary"
end

Then(/^I should see the "([^"]*)" role has been assigned to the worldwide organisation "([^"]*)"$/) do |role, title|
@worldwide_organisation = EditionableWorldwideOrganisation.find_by(title:)
expect(@worldwide_organisation.roles.first.name).to eq(role)
end
6 changes: 6 additions & 0 deletions test/factories/edition_roles.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
FactoryBot.define do
factory :edition_role do
edition
role
end
end
6 changes: 6 additions & 0 deletions test/factories/editionable_worldwide_organisations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
news_article.world_locations << build(:world_location)
end
end

trait(:with_role) do
after :create do |organisation, _evaluator|
organisation.roles << create(:ambassador_role)
end
end
end

factory :draft_editionable_worldwide_organisation, parent: :editionable_worldwide_organisation, traits: [:draft]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class Admin::EditionableWorldwideOrganisationsControllerTest < ActionController:
should_allow_creating_of :editionable_worldwide_organisation
should_allow_editing_of :editionable_worldwide_organisation

should_allow_association_between_roles_and :editionable_worldwide_organisation
should_allow_association_between_world_locations_and :editionable_worldwide_organisation

test "actions are forbidden when the editionable_worldwide_organisations feature flag is disabled" do
Expand All @@ -23,6 +24,9 @@ class Admin::EditionableWorldwideOrganisationsControllerTest < ActionController:
end

def controller_attributes_for(edition_type, attributes = {})
super.merge(world_location_ids: [create(:world_location).id])
super.merge(
role_ids: [create(:role).id],
world_location_ids: [create(:world_location).id],
)
end
end
76 changes: 76 additions & 0 deletions test/support/admin_edition_roles_behaviour.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
module AdminEditionRolesBehaviour
extend ActiveSupport::Concern

module ClassMethods
def should_allow_association_between_roles_and(document_type)
edition_class = class_for(document_type)

view_test "new displays document form with roles field" do
get :new

assert_select "form#new_edition" do
assert_select "label[for=edition_roles]", text: "Roles"
assert_select "#edition_roles" do |elements|
assert_equal 1, elements.length
end
end
end

test "creating should create a new document with roles" do
role1 = create(:role)
role2 = create(:role)
attributes = controller_attributes_for(document_type)

post :create,
params: {
edition: attributes.merge(
role_ids: [role1.id, role2.id],
),
}

assert document = edition_class.last
assert_equal [role1, role2], document.roles
end

view_test "edit displays document form with roles field" do
edition = create(document_type) # rubocop:disable Rails/SaveBang
get :edit, params: { id: edition }

assert_select "form#edit_edition" do
assert_select "label[for=edition_roles]", text: "Roles"

assert_select "#edition_roles" do |elements|
assert_equal 1, elements.length
end
end
end

test "updating should save modified document attributes with roles" do
role1 = create(:role)
role2 = create(:role)
document = create(document_type, roles: [role2])

put :update,
params: { id: document,
edition: {
role_ids: [role1.id],
} }

document = document.reload
assert_equal [role1], document.roles
end

view_test "updating a stale document should render edit page with conflicting document and its roles" do
document = create(document_type) # rubocop:disable Rails/SaveBang
lock_version = document.lock_version
document.touch

put :update, params: { id: document, edition: { lock_version: } }

assert_select ".conflict" do
assert_select "h2", "Roles"
end
end
end
end
end
1 change: 1 addition & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ class ActionController::TestCase
include AdminControllerTestHelpers
include AdminEditionControllerTestHelpers
include AdminEditionControllerScheduledPublishingTestHelpers
include AdminEditionRolesBehaviour
include AdminEditionWorldLocationsBehaviour
include DocumentControllerTestHelpers
include ControllerTestHelpers
Expand Down
17 changes: 17 additions & 0 deletions test/unit/app/models/edition_role_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
require "test_helper"

class EditionRoleTest < ActiveSupport::TestCase
include ActionDispatch::TestProcess

test "should be invalid without an edition" do
edition_role = build(:edition_role, edition: nil)
assert_not edition_role.valid?
assert edition_role.errors[:edition].present?
end

test "should be invalid without an role" do
edition_role = build(:edition_role, role: nil)
assert_not edition_role.valid?
assert edition_role.errors[:role].present?
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def present(...)
end

test "presents a Worldwide Organisation ready for adding to the publishing API" do
worldwide_org = create(:editionable_worldwide_organisation)
worldwide_org = create(:editionable_worldwide_organisation, :with_role)

public_path = worldwide_org.public_path

Expand All @@ -30,6 +30,7 @@ def present(...)
}

expected_links = {
roles: worldwide_org.roles.map(&:content_id),
sponsoring_organisations: worldwide_org.organisations.map(&:content_id),
world_locations: worldwide_org.world_locations.map(&:content_id),
}
Expand Down

0 comments on commit cd09bb6

Please sign in to comment.