From 25ce9fdbf35d44fc7dbda40e3cd48ac509ddb313 Mon Sep 17 00:00:00 2001 From: Derek Ekins Date: Fri, 15 Jan 2021 10:37:19 +0000 Subject: [PATCH] Add publication_status to initiative (#99) * show publication status drop down, restrict results to only show published initiatives * disable validation when saving in draft mode * don't allow editing of archived initiatives * only show published initiatives * update schema * if an initiative has been rejected then don't allow regular users to change the publication status * fixed linting issues --- .gitignore | 2 + app/controllers/districts_controller.rb | 2 +- app/controllers/home_controller.rb | 2 +- app/controllers/initiatives_controller.rb | 29 +++++++---- app/controllers/parishes_controller.rb | 2 +- app/controllers/wards_controller.rb | 2 +- app/helpers/initiatives_helper.rb | 8 ++++ app/models/initiative.rb | 48 ++++++++++++------- app/views/initiatives/_form.html.erb | 7 +++ app/views/initiatives/index.html.erb | 2 +- app/views/parishes/show.html.erb | 12 ++--- config/rails_best_practices.yml | 2 +- ...15_add_publication_status_to_initiative.rb | 17 +++++++ db/schema.rb | 9 ++-- frontend/packs/initiative_solution_picker.js | 16 +++---- frontend/styles/components/initiative.css | 4 ++ lib/tasks/db_update.rake | 28 +++++++++++ .../initiatives_controller_test.rb | 37 +++++++++++++- test/fixtures/initiatives.yml | 2 + test/models/initiative_test.rb | 4 ++ 20 files changed, 184 insertions(+), 51 deletions(-) create mode 100644 db/migrate/20200616193415_add_publication_status_to_initiative.rb create mode 100644 lib/tasks/db_update.rake diff --git a/.gitignore b/.gitignore index 3c5ca93..03b6b8b 100644 --- a/.gitignore +++ b/.gitignore @@ -38,3 +38,5 @@ yarn-debug.log* # these images are copied in at install time /public/leaflet +*.dump +.envrc diff --git a/app/controllers/districts_controller.rb b/app/controllers/districts_controller.rb index 439d34e..a82f182 100644 --- a/app/controllers/districts_controller.rb +++ b/app/controllers/districts_controller.rb @@ -6,7 +6,7 @@ class DistrictsController < ApplicationController def show @district = District.find(params['id']) initiatives = - Initiative.includes(parish: %i[ward]).where( + Initiative.published.includes(parish: %i[ward]).where( parishes: { wards: { district_id: @district.id } } ) @map_data = MapData.new(initiatives) diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index cbdae07..0ae4187 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -7,7 +7,7 @@ class HomeController < ApplicationController def index @initiatives_json = Initiative.approved.to_json - initiatives = Initiative.all + initiatives = Initiative.published @map_data = MapData.new(initiatives) @sectors = Sector.all @districts = District.all diff --git a/app/controllers/initiatives_controller.rb b/app/controllers/initiatives_controller.rb index 9c686fd..6f12d9e 100644 --- a/app/controllers/initiatives_controller.rb +++ b/app/controllers/initiatives_controller.rb @@ -8,11 +8,16 @@ class InitiativesController < ApplicationController helper_method :can_edit? def can_edit?(initiative) - initiative.owner == current_user + initiative.owner == current_user || current_user&.role == 'admin' end def index - @initiatives = Initiative.all + current_users_initiatives = current_user&.initiatives || [] + @initiatives = if current_user&.role == 'admin' + Initiative.all.sort_by(&:name) + else + (Initiative.published + current_users_initiatives).uniq.sort_by(&:name) + end end def show @@ -37,12 +42,11 @@ def edit; end # rubocop:disable Metrics/MethodLength def create create_proposed_solutions - @initiative = Initiative.new(initiative_params) - @initiative.owner = current_user + @initiative = Initiative.new(initiative_params.merge(owner: current_user)) find_or_create_group @initiative.update_location_from_postcode - if @initiative.save + if @initiative.save(validate: @initiative.publication_status != 'draft') redirect_to edit_initiative_path(@initiative), notice: 'Initiative was successfully created.' else @@ -51,15 +55,23 @@ def create end # rubocop:enable Metrics/MethodLength - # rubocop:disable Metrics/MethodLength + # rubocop:disable Metrics/MethodLength, Metrics/AbcSize, Metrics/PerceivedComplexity, Metrics/CyclomaticComplexity def update + publication_status = @initiative.publication_status + if publication_status == 'archived' + redirect_to initiatives_path, notice: "'#{@initiative.name}' has been archived and cannot be edited" + return + end + + initiative_params.delete(:publication_status) if publication_status == 'rejected' && current_user.role != 'admin' + clear_solutions_and_themes && create_proposed_solutions images = initiative_params.delete 'images' find_or_create_group @initiative.assign_attributes initiative_params @initiative.update_location_from_postcode - if @initiative.save + if @initiative.save(validate: publication_status != 'draft') @initiative.images.attach images if images redirect_to edit_initiative_path(@initiative), notice: 'Initiative was successfully updated.' @@ -67,7 +79,7 @@ def update render :edit end end - # rubocop:enable Metrics/MethodLength + # rubocop:enable Metrics/MethodLength, Metrics/AbcSize, Metrics/PerceivedComplexity, Metrics/CyclomaticComplexity private @@ -182,6 +194,7 @@ def initiative_params :contact_phone, :partner_groups_role, :status_id, + :publication_status, :consent_to_share, :related_initiatives, :administrative_notes, diff --git a/app/controllers/parishes_controller.rb b/app/controllers/parishes_controller.rb index c966837..234157c 100644 --- a/app/controllers/parishes_controller.rb +++ b/app/controllers/parishes_controller.rb @@ -7,7 +7,7 @@ def show @parish = Parish.find(params['id']) @ward = @parish.ward @district = @ward.district - initiatives = Initiative.where(parish_id: @parish.id) + initiatives = Initiative.published.where(parish_id: @parish.id) @map_data = MapData.new(initiatives) @sectors = Sector.all end diff --git a/app/controllers/wards_controller.rb b/app/controllers/wards_controller.rb index 487396e..a87f884 100644 --- a/app/controllers/wards_controller.rb +++ b/app/controllers/wards_controller.rb @@ -6,7 +6,7 @@ class WardsController < ApplicationController def show @ward = Ward.find(params['id']) initiatives = - Initiative.includes(parish: %i[ward]).where( + Initiative.published.includes(parish: %i[ward]).where( parishes: { ward_id: @ward.id } ) @district = @ward.district diff --git a/app/helpers/initiatives_helper.rb b/app/helpers/initiatives_helper.rb index 36ee937..30888f4 100644 --- a/app/helpers/initiatives_helper.rb +++ b/app/helpers/initiatives_helper.rb @@ -18,4 +18,12 @@ def solutions_as_json(initiative) solution_map.to_json end + + def publication_statuses + statuses = Initiative.publication_statuses.to_h + statuses.reject! { |status| %w[rejected archived].include?(status) } unless user_is_admin? + statuses.map do |key, value| + [value.titlecase, key] + end + end end diff --git a/app/models/initiative.rb b/app/models/initiative.rb index cc6d3fa..3e29f36 100644 --- a/app/models/initiative.rb +++ b/app/models/initiative.rb @@ -4,17 +4,26 @@ # rubocop:disable Metrics/ClassLength class Initiative < ApplicationRecord + after_initialize :set_default_location + after_initialize :set_default_publication_status + belongs_to :owner, class_name: 'User' belongs_to :lead_group, class_name: 'Group' belongs_to :status, class_name: 'InitiativeStatus' belongs_to :parish + delegate :name, prefix: true, to: :status delegate :name, prefix: true, to: :lead_group - after_initialize :set_default_location + delegate :ward, to: :parish + delegate :district, to: :ward + delegate :county, to: :district + delegate :region, to: :county + has_many :solutions, class_name: 'InitiativeSolution', dependent: :destroy has_many :themes, class_name: 'InitiativeTheme', dependent: :destroy has_many_attached :images has_many :websites, class_name: 'InitiativeWebsite', dependent: :destroy + accepts_nested_attributes_for :solutions accepts_nested_attributes_for :themes accepts_nested_attributes_for :lead_group @@ -29,14 +38,13 @@ class Initiative < ApplicationRecord :contact_email, :contact_phone, :lead_group, + :publication_status, presence: true validate :at_least_one_solution_or_theme validate :validate_postcode - def website_empty?(attributes) - attributes['url'].blank? - end + enum publication_status: { draft: 'draft', published: 'published', archived: 'archived', rejected: 'rejected' } def validate_postcode ukpc = UKPostcode.parse(postcode) @@ -51,11 +59,6 @@ def at_least_one_solution_or_theme errors.add(:solution, 'at least one equired') if solutions.empty? && themes.empty? end - def set_default_location - self.latitude ||= 51.742 - self.longitude ||= -2.222 - end - def public_attributes if consent_to_share attributes @@ -65,6 +68,8 @@ def public_attributes end def fetch_location + return unless postcode + postcodes_url = "https://api.postcodes.io/postcodes/#{postcode.delete(' ')}" json_response = Net::HTTP.get(URI(postcodes_url)) JSON.parse(json_response)['result'] @@ -91,21 +96,13 @@ def create_location(location) end def self.approved - Initiative.all.map(&:to_public_initiative) + Initiative.published.map(&:to_public_initiative) end def to_public_initiative PublicInitiative.new(self) end - delegate :ward, to: :parish - - delegate :district, to: :ward - - delegate :county, to: :district - - delegate :region, to: :county - def location return unless parish @@ -129,5 +126,20 @@ def location_attributes } end # rubocop:enable Metrics/MethodLength + + private + + def set_default_location + self.latitude ||= 51.742 + self.longitude ||= -2.222 + end + + def set_default_publication_status + self.publication_status = 'draft' if publication_status.blank? + end + + def website_empty?(attributes) + attributes['url'].blank? + end end # rubocop:enable Metrics/ClassLength diff --git a/app/views/initiatives/_form.html.erb b/app/views/initiatives/_form.html.erb index 5f80ca2..8c4d484 100644 --- a/app/views/initiatives/_form.html.erb +++ b/app/views/initiatives/_form.html.erb @@ -170,6 +170,13 @@ <%= f.text_area :administrative_notes %> <% end %> + <% if publication_statuses.map {|s| s[1]}.include?(initiative.publication_status) %> + <%= f.form_field :publication_status, required: true do %> + <%= f.label :publication_status, 'Publication Status' %> + <%= f.select :publication_status, publication_statuses %> + <% end %> + <% end %> +
<%= f.submit class: 'Button Button--success Button--large' %>
diff --git a/app/views/initiatives/index.html.erb b/app/views/initiatives/index.html.erb index 6f36cdd..15fcb0f 100644 --- a/app/views/initiatives/index.html.erb +++ b/app/views/initiatives/index.html.erb @@ -25,7 +25,7 @@ <% @initiatives.each do |initiative| %> - + <%= link_to initiative.name, initiative_path(initiative) %> <%= initiative.location %> <%= initiative.status_name %> diff --git a/app/views/parishes/show.html.erb b/app/views/parishes/show.html.erb index 3423f33..e99d4f0 100644 --- a/app/views/parishes/show.html.erb +++ b/app/views/parishes/show.html.erb @@ -14,12 +14,12 @@ ] do %> <% end %> - diff --git a/config/rails_best_practices.yml b/config/rails_best_practices.yml index e9ba5ee..acd1a4e 100644 --- a/config/rails_best_practices.yml +++ b/config/rails_best_practices.yml @@ -13,7 +13,7 @@ MoveCodeIntoControllerCheck: {} MoveCodeIntoHelperCheck: { array_count: 3 } MoveCodeIntoModelCheck: { use_count: 2 } MoveFinderToNamedScopeCheck: {} -MoveModelLogicIntoModelCheck: { use_count: 4 } +MoveModelLogicIntoModelCheck: { use_count: 6 } NeedlessDeepNestingCheck: { nested_count: 2 } NotRescueExceptionCheck: {} NotUseDefaultRouteCheck: {} diff --git a/db/migrate/20200616193415_add_publication_status_to_initiative.rb b/db/migrate/20200616193415_add_publication_status_to_initiative.rb new file mode 100644 index 0000000..262b0b6 --- /dev/null +++ b/db/migrate/20200616193415_add_publication_status_to_initiative.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddPublicationStatusToInitiative < ActiveRecord::Migration[6.0] + def change + add_column :initiatives, :publication_status, :string + add_index :initiatives, :publication_status + + reversible do |dir| + dir.up do + ActiveRecord::Base.connection.execute("update initiatives set publication_status = 'published'") + end + end + + change_column_null :initiatives, :lead_group_id, true + change_column_null :initiatives, :status_id, true + end +end diff --git a/db/schema.rb b/db/schema.rb index cb91867..5562ff0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_01_18_113424) do +ActiveRecord::Schema.define(version: 2020_06_16_193415) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -127,12 +127,12 @@ t.string "name" t.string "description_further_information" t.integer "carbon_saving_amount" - t.bigint "lead_group_id", null: false + t.bigint "lead_group_id" t.string "contact_name" t.string "contact_email" t.string "contact_phone" t.string "partner_groups_role" - t.bigint "status_id", null: false + t.bigint "status_id" t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false t.float "latitude" @@ -146,11 +146,12 @@ t.string "administrative_notes" t.boolean "carbon_saving_anticipated", default: false, null: false t.boolean "carbon_saving_quantified", default: false, null: false - t.boolean "draft", default: false, null: false t.bigint "owner_id" + t.string "publication_status" t.index ["lead_group_id"], name: "index_initiatives_on_lead_group_id" t.index ["owner_id"], name: "index_initiatives_on_owner_id" t.index ["parish_id"], name: "index_initiatives_on_parish_id" + t.index ["publication_status"], name: "index_initiatives_on_publication_status" t.index ["status_id"], name: "index_initiatives_on_status_id" end diff --git a/frontend/packs/initiative_solution_picker.js b/frontend/packs/initiative_solution_picker.js index 7928030..d2c7b8d 100644 --- a/frontend/packs/initiative_solution_picker.js +++ b/frontend/packs/initiative_solution_picker.js @@ -97,8 +97,8 @@ class SolutionPicker { class="AddInitiativeResults-item" onclick={() => this.addSolution(solution)} > - {solution.sector} > {solution.theme} > {solution.class} >{" "} - {solution.solution} + {solution.sector} > {solution.theme} > {solution.class}{" "} + > {solution.solution} ); })} @@ -125,7 +125,7 @@ class SolutionPicker { function renderSector() { if (self.navigation.sector) { return [ -  > , +  > , self.navigate({ sector: self.navigation.sector })} > @@ -137,7 +137,7 @@ class SolutionPicker { function renderTheme() { if (self.navigation.theme) { return [ -  > , +  gt; , self.navigate({ theme: self.navigation.theme })}> {self.navigation.theme.name} @@ -147,7 +147,7 @@ class SolutionPicker { function renderSolutionClass() { if (self.navigation.solutionClass) { return [ -  > , +  > , self.navigate({ solutionClass: self.navigation.solutionClass }) @@ -369,7 +369,7 @@ class SolutionPicker { return (
  • - {theme.sector} > {theme.name} + {theme.sector} > {theme.name} - {solution.sector} > {solution.theme} > {solution.class} >{" "} - {solution.solution} + {solution.sector} > {solution.theme} > {solution.class}{" "} + > {solution.solution} e + puts '----------------------------------------------------' + puts 'IGNORING ERRORS! by pg_restore (Double check the above errors are OK!)' + puts e.message + puts '----------------------------------------------------' + end + + sh 'rails db:migrate' + end +end diff --git a/test/controllers/initiatives_controller_test.rb b/test/controllers/initiatives_controller_test.rb index 653c2bb..b81112d 100644 --- a/test/controllers/initiatives_controller_test.rb +++ b/test/controllers/initiatives_controller_test.rb @@ -54,6 +54,39 @@ def header_image assert_redirected_to edit_initiative_path(Initiative.last) end + test 'create draft initiative' do + sign_in_as :georgie + assert_difference('Initiative.count') do + post initiatives_url, params: { initiative: { name: 'draft init', publication_status: 'draft' } } + end + initiative = Initiative.last + assert_equal 'draft init', initiative.name + + assert_redirected_to edit_initiative_path(Initiative.last) + end + + test 'archived initiative cannot be edited' do + @initiative.update!(publication_status: 'archived') + + sign_in_as :georgie + patch initiative_url(@initiative), + params: update_params(@initiative) + + assert_redirected_to initiatives_path + end + + test 'pubication status ignored on rejected initiative' do + @initiative.update!(publication_status: 'rejected') + + sign_in_as :georgie + VCR.use_cassette('valid_postcode') do + patch initiative_url(@initiative), + params: update_params(@initiative, publication_status: 'published') + end + + assert_equal 'rejected', @initiative.reload.publication_status + end + test 'create initiative and lead group' do sign_in_as :georgie lead_group = { @@ -243,7 +276,8 @@ def create_params(initiative, lead_group: nil, images: nil, solutions: nil, end def update_params(initiative, images: nil, solutions: nil, - carbon_saving_anticipated: false, carbon_saving_quantified: false, carbon_saving_amount: nil) + carbon_saving_anticipated: false, carbon_saving_quantified: false, + carbon_saving_amount: nil, publication_status: nil) solutions ||= default_solutions { initiative: { @@ -263,6 +297,7 @@ def update_params(initiative, images: nil, solutions: nil, description_how: initiative.description_how, images: images, consent_to_share: true, + publication_status: publication_status || initiative.publication_status, solutions_attributes: solutions, administrative_notes: 'updated notes', websites_attributes: [ diff --git a/test/fixtures/initiatives.yml b/test/fixtures/initiatives.yml index 12f6ba7..0541af3 100644 --- a/test/fixtures/initiatives.yml +++ b/test/fixtures/initiatives.yml @@ -16,6 +16,7 @@ fruit_exchange: contact_email: info@downtoearthstroud.co.uk status: operational consent_to_share: true + publication_status: published brians_initiative: owner: brian @@ -33,3 +34,4 @@ brians_initiative: contact_email: info@example.com status: operational consent_to_share: true + publication_status: published diff --git a/test/models/initiative_test.rb b/test/models/initiative_test.rb index 73b1ffa..e578c18 100644 --- a/test/models/initiative_test.rb +++ b/test/models/initiative_test.rb @@ -3,6 +3,10 @@ require 'test_helper' class InitiativeTest < ActiveSupport::TestCase + test 'new initiative defaults to draft' do + assert_equal 'draft', Initiative.new.publication_status + end + test 'approved json' do initiatives = Initiative.approved.map do |initiative|