From 375700691cc0e540fefd800a4b5345404298dc13 Mon Sep 17 00:00:00 2001 From: "Eric L. Truitte, MBA" Date: Sat, 13 Oct 2018 20:05:21 -0400 Subject: [PATCH] Adding trash and making Shelters and Distribution Points 'trashable' so that they can be 'deleted' Closes #54 --- .../distribution_points_controller.rb | 7 +- app/controllers/shelters_controller.rb | 7 +- app/models/concerns/geocodable.rb | 2 +- app/models/concerns/trashable.rb | 21 ++++++ app/models/distribution_point.rb | 3 +- app/models/shelter.rb | 3 +- app/models/trash.rb | 16 ++++ app/views/distribution_points/show.html.erb | 6 +- app/views/shelters/show.html.erb | 6 +- db/migrate/20181013222410_create_trashes.rb | 11 +++ db/structure.sql | 75 ++++++++++++++++++- .../distribution_points_controller_test.rb | 27 +++++++ test/controllers/shelters_controller_test.rb | 27 +++++++ 13 files changed, 203 insertions(+), 8 deletions(-) create mode 100644 app/models/concerns/trashable.rb create mode 100644 app/models/trash.rb create mode 100644 db/migrate/20181013222410_create_trashes.rb diff --git a/app/controllers/distribution_points_controller.rb b/app/controllers/distribution_points_controller.rb index 2be4772..0d9e2ac 100644 --- a/app/controllers/distribution_points_controller.rb +++ b/app/controllers/distribution_points_controller.rb @@ -1,5 +1,5 @@ class DistributionPointsController < ApplicationController - before_action :authenticate_admin!, only: [:archive, :unarchive] + before_action :authenticate_admin!, only: [:archive, :unarchive, :destroy] before_action :set_headers, except: [:index] before_action :set_index_headers, only: [:index] before_action :set_distribution_point, only: [:show, :edit, :update, :destroy, :archive, :unarchive] @@ -64,6 +64,11 @@ def update end def destroy + if @distribution_point.trash!(current_user, params[:reason]) + redirect_to distribution_points_path, notice: "'#{@distribution_point.facility_name}' has been moved to the trash." + else + redirect_to @distribution_point, notice: "Something went wrong, '#{@distribution_point.facility_name}' has not been trashed." + end end def archived diff --git a/app/controllers/shelters_controller.rb b/app/controllers/shelters_controller.rb index a6dc1ab..a448668 100644 --- a/app/controllers/shelters_controller.rb +++ b/app/controllers/shelters_controller.rb @@ -1,5 +1,5 @@ class SheltersController < ApplicationController - before_action :authenticate_admin!, only: [:archive, :unarchive] + before_action :authenticate_admin!, only: [:archive, :unarchive, :destroy] before_action :set_headers, except: [:index] before_action :set_index_headers, only: [:index] before_action :set_shelter, only: [:show, :edit, :update, :destroy, :archive, :unarchive] @@ -64,6 +64,11 @@ def update end def destroy + if @shelter.trash!(current_user, params[:reason]) + redirect_to shelters_path, notice: "'#{@shelter.shelter}' has been moved to the trash." + else + redirect_to @shelter, notice: "Something went wrong, '#{@shelter.shelter}' has not been trashed." + end end def archived diff --git a/app/models/concerns/geocodable.rb b/app/models/concerns/geocodable.rb index c0f62ef..357cc8d 100644 --- a/app/models/concerns/geocodable.rb +++ b/app/models/concerns/geocodable.rb @@ -10,7 +10,7 @@ module Geocodable # latitude, longitude, address, city, state, zip, county included do - after_commit :schedule_reverse_geocode + after_commit :schedule_reverse_geocode, on: [:create, :update] scope :incomplete_geocoding, -> { where(<<~SQL county IS NULL OR TRIM(county) = '' OR diff --git a/app/models/concerns/trashable.rb b/app/models/concerns/trashable.rb new file mode 100644 index 0000000..5a1abdc --- /dev/null +++ b/app/models/concerns/trashable.rb @@ -0,0 +1,21 @@ +module Trashable + extend ActiveSupport::Concern + + included do + has_one :trash, as: :trashable + end + + def trash!(user, reason = nil) + transaction do + trash = Trash.create(trashable: self, + user: user, + data: self.attributes, + reason: reason) + unless trash && self.destroy + trash = nil + raise ActiveRecord::Rollback + end + end + trash + end +end diff --git a/app/models/distribution_point.rb b/app/models/distribution_point.rb index 13b54ea..32b3c56 100644 --- a/app/models/distribution_point.rb +++ b/app/models/distribution_point.rb @@ -1,5 +1,6 @@ class DistributionPoint < ApplicationRecord include Geocodable + include Trashable default_scope { where(archived: false) } @@ -37,7 +38,7 @@ class DistributionPoint < ApplicationRecord has_many :drafts, as: :record - after_commit do + after_commit on: [:create, :update] do DistributionPointUpdateNotifierJob.perform_later self end diff --git a/app/models/shelter.rb b/app/models/shelter.rb index 5225e27..437ac91 100644 --- a/app/models/shelter.rb +++ b/app/models/shelter.rb @@ -1,5 +1,6 @@ class Shelter < ApplicationRecord include Geocodable + include Trashable default_scope { where(active: true) } @@ -55,7 +56,7 @@ class Shelter < ApplicationRecord has_many :drafts, as: :record - after_commit do + after_commit on: [:create, :update] do ShelterUpdateNotifierJob.perform_later self end diff --git a/app/models/trash.rb b/app/models/trash.rb new file mode 100644 index 0000000..6854925 --- /dev/null +++ b/app/models/trash.rb @@ -0,0 +1,16 @@ +class Trash < ApplicationRecord + belongs_to :user + belongs_to :trashable, polymorphic: true, optional: true + + def restore! + restoring = nil + transaction do + restoring = trashable_type.constantize.create(data) + unless restoring && self.destroy + restoring = nil + raise ActiveRecord::Rollback + end + end + restoring + end +end diff --git a/app/views/distribution_points/show.html.erb b/app/views/distribution_points/show.html.erb index 56e4717..082046f 100644 --- a/app/views/distribution_points/show.html.erb +++ b/app/views/distribution_points/show.html.erb @@ -18,7 +18,11 @@ method: :post, data: { confirm: 'Are you sure?' }, class: 'button button-clear' %> - <% end %> + <% end %> | + <%= link_to 'Delete', @distribution_point, + method: :delete, + data: { confirm: 'Are you sure?' }, + class: 'button button-clear' %> <% end %>
diff --git a/app/views/shelters/show.html.erb b/app/views/shelters/show.html.erb index 749f1fa..22e26cf 100644 --- a/app/views/shelters/show.html.erb +++ b/app/views/shelters/show.html.erb @@ -17,7 +17,11 @@ method: :delete, data: { confirm: 'Are you sure?' }, class: 'button button-clear' %> - <% end %> + <% end %> | + <%= link_to 'Delete', @shelter, + method: :delete, + data: { confirm: 'Are you sure?' }, + class: 'button button-clear' %> <% end %>
diff --git a/db/migrate/20181013222410_create_trashes.rb b/db/migrate/20181013222410_create_trashes.rb new file mode 100644 index 0000000..f30a2c5 --- /dev/null +++ b/db/migrate/20181013222410_create_trashes.rb @@ -0,0 +1,11 @@ +class CreateTrashes < ActiveRecord::Migration[5.1] + def change + create_table :trashes do |t| + t.references :trashable, polymorphic: true, null: false + t.jsonb :data + t.references :user, foreign_key: true + t.text :reason + t.timestamps + end + end +end diff --git a/db/structure.sql b/db/structure.sql index 38f9265..16e2844 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -521,6 +521,41 @@ CREATE SEQUENCE public.shelters_id_seq ALTER SEQUENCE public.shelters_id_seq OWNED BY public.shelters.id; +-- +-- Name: trashes; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.trashes ( + id bigint NOT NULL, + trashable_type character varying NOT NULL, + trashable_id bigint NOT NULL, + data jsonb, + user_id bigint, + reason text, + created_at timestamp without time zone NOT NULL, + updated_at timestamp without time zone NOT NULL +); + + +-- +-- Name: trashes_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE public.trashes_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: trashes_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE public.trashes_id_seq OWNED BY public.trashes.id; + + -- -- Name: users; Type: TABLE; Schema: public; Owner: - -- @@ -679,6 +714,13 @@ ALTER TABLE ONLY public.pages ALTER COLUMN id SET DEFAULT nextval('public.pages_ ALTER TABLE ONLY public.shelters ALTER COLUMN id SET DEFAULT nextval('public.shelters_id_seq'::regclass); +-- +-- Name: trashes id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.trashes ALTER COLUMN id SET DEFAULT nextval('public.trashes_id_seq'::regclass); + + -- -- Name: users id; Type: DEFAULT; Schema: public; Owner: - -- @@ -797,6 +839,14 @@ ALTER TABLE ONLY public.shelters ADD CONSTRAINT shelters_pkey PRIMARY KEY (id); +-- +-- Name: trashes trashes_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.trashes + ADD CONSTRAINT trashes_pkey PRIMARY KEY (id); + + -- -- Name: users users_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -862,6 +912,20 @@ CREATE INDEX index_drafts_on_denied_by_id ON public.drafts USING btree (denied_b CREATE INDEX index_drafts_on_record_type_and_record_id ON public.drafts USING btree (record_type, record_id); +-- +-- Name: index_trashes_on_trashable_type_and_trashable_id; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_trashes_on_trashable_type_and_trashable_id ON public.trashes USING btree (trashable_type, trashable_id); + + +-- +-- Name: index_trashes_on_user_id; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_trashes_on_user_id ON public.trashes USING btree (user_id); + + -- -- Name: index_users_on_confirmation_token; Type: INDEX; Schema: public; Owner: - -- @@ -899,6 +963,14 @@ ALTER TABLE ONLY public.drafts ADD CONSTRAINT fk_rails_687f4d113b FOREIGN KEY (denied_by_id) REFERENCES public.users(id); +-- +-- Name: trashes fk_rails_7d97d072f5; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.trashes + ADD CONSTRAINT fk_rails_7d97d072f5 FOREIGN KEY (user_id) REFERENCES public.users(id); + + -- -- PostgreSQL database dump complete -- @@ -940,6 +1012,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20170910045633'), ('20180914132709'), ('20180915035427'), -('20180919043949'); +('20180919043949'), +('20181013222410'); diff --git a/test/controllers/distribution_points_controller_test.rb b/test/controllers/distribution_points_controller_test.rb index b07f821..51f4aac 100644 --- a/test/controllers/distribution_points_controller_test.rb +++ b/test/controllers/distribution_points_controller_test.rb @@ -127,4 +127,31 @@ class DistributionPointsControllerTest < ActionDispatch::IntegrationTest get drafts_distribution_points_path assert_response :success end + + test "guests may not delete" do + pod = distribution_points(:nrg) + sign_in users(:guest) + delete distribution_point_path(pod) + assert_response :redirect + assert_redirected_to root_path + end + + test "admins can delete" do + pod = distribution_points(:nrg) + sign_in users(:admin) + delete distribution_point_path(pod) + assert_response :redirect + assert_redirected_to distribution_points_path + end + + test "deleted items get moved to trash" do + pod = distribution_points(:nrg) + expected_pods = DistributionPoint.count - 1 + expected_trash = Trash.count + 1 + sign_in users(:admin) + delete distribution_point_path(pod) + assert_response :redirect + assert_equal expected_pods, DistributionPoint.count + assert_equal expected_trash, Trash.count + end end diff --git a/test/controllers/shelters_controller_test.rb b/test/controllers/shelters_controller_test.rb index 2270e15..04706cd 100644 --- a/test/controllers/shelters_controller_test.rb +++ b/test/controllers/shelters_controller_test.rb @@ -123,4 +123,31 @@ class SheltersControllerTest < ActionDispatch::IntegrationTest get drafts_shelters_path assert_response :success end + + test "guests may not delete" do + shelter = shelters(:nrg) + sign_in users(:guest) + delete shelter_path(shelter) + assert_response :redirect + assert_redirected_to root_path + end + + test "admins can delete" do + shelter = shelters(:nrg) + sign_in users(:admin) + delete shelter_path(shelter) + assert_response :redirect + assert_redirected_to shelters_path + end + + test "deleted items get moved to trash" do + shelter = shelters(:nrg) + expected_shelters = Shelter.count - 1 + expected_trash = Trash.count + 1 + sign_in users(:admin) + delete shelter_path(shelter) + assert_response :redirect + assert_equal expected_shelters, Shelter.count + assert_equal expected_trash, Trash.count + end end