diff --git a/CHANGELOG.md b/CHANGELOG.md index 8705d0a79..7b28fa834 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ - Add more context data to sentry reports - Improved SSO testing - Ability to map the external ID from the SSO +- Ability to soft-destroy a reserved event - Fix a bug: unable to run task fix_invoice_item when some invoice items are associated with errors - Fix a bug: invalid event date reported when the timezone in before UTC - Fix a bug: unable to run accounting export if a line label was not defined diff --git a/app/controllers/api/events_controller.rb b/app/controllers/api/events_controller.rb index 9937d63bc..bd43a0245 100644 --- a/app/controllers/api/events_controller.rb +++ b/app/controllers/api/events_controller.rb @@ -36,7 +36,8 @@ class API::EventsController < API::ApiController limit = params[:limit] @events = Event.includes(:event_image, :event_files, :availability, :category) .where('events.nb_total_places != -1 OR events.nb_total_places IS NULL') - .order('availabilities.start_at ASC').references(:availabilities) + .where(deleted_at: nil) + .order('availabilities.start_at').references(:availabilities) .limit(limit) @events = case Setting.get('upcoming_events_shown') @@ -49,7 +50,9 @@ class API::EventsController < API::ApiController end end - def show; end + def show + head :not_found if @event.deleted_at + end def create authorize Event diff --git a/app/controllers/open_api/v1/events_controller.rb b/app/controllers/open_api/v1/events_controller.rb index 247ef6685..d9c234bab 100644 --- a/app/controllers/open_api/v1/events_controller.rb +++ b/app/controllers/open_api/v1/events_controller.rb @@ -8,6 +8,7 @@ class OpenAPI::V1::EventsController < OpenAPI::V1::BaseController def index @events = Event.includes(:event_image, :event_files, :availability, :category) + .where(deleted_at: nil) @events = if upcoming @events.references(:availabilities) .where('availabilities.end_at >= ?', DateTime.current) diff --git a/app/models/event.rb b/app/models/event.rb index 4cd66cbb7..d9d3aba18 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -53,13 +53,12 @@ class Event < ApplicationRecord .references(:availabilities) end - def safe_destroy - reservations = Reservation.where(reservable_type: 'Event', reservable_id: id) - if reservations.size.zero? - destroy - else - false - end + def destroyable? + Reservation.where(reservable_type: 'Event', reservable_id: id).count.zero? + end + + def soft_destroy! + update(deleted_at: DateTime.current) end ## diff --git a/app/policies/event_policy.rb b/app/policies/event_policy.rb index 8e350a944..7563498d3 100644 --- a/app/policies/event_policy.rb +++ b/app/policies/event_policy.rb @@ -6,12 +6,16 @@ class EventPolicy < ApplicationPolicy class Scope < Scope def resolve if user.nil? || (user && !user.admin? && !user.manager?) - scope.includes(:event_image, :event_files, :availability, :category, :event_price_categories, :age_range, :events_event_themes, :event_themes) + scope.includes(:event_image, :event_files, :availability, :category, :event_price_categories, :age_range, :events_event_themes, + :event_themes) .where('availabilities.start_at >= ?', DateTime.current) + .where(deleted_at: nil) .order('availabilities.start_at ASC') .references(:availabilities) else - scope.includes(:event_image, :event_files, :availability, :category, :event_price_categories, :age_range, :events_event_themes, :event_themes) + scope.includes(:event_image, :event_files, :availability, :category, :event_price_categories, :age_range, :events_event_themes, + :event_themes) + .where(deleted_at: nil) .references(:availabilities) end end diff --git a/app/services/event_service.rb b/app/services/event_service.rb index dcb7ea0af..43d799aeb 100644 --- a/app/services/event_service.rb +++ b/app/services/event_service.rb @@ -70,8 +70,9 @@ class EventService end events.each do |e| - # we use double negation because safe_destroy can return either a boolean (false) or an Availability (in case of delete success) - results.push status: !!e.safe_destroy, event: e # rubocop:disable Style/DoubleNegation + method = e.destroyable? ? :destroy : :soft_destroy! + # we use double negation because destroy can return either a boolean (false) or an Event (in case of delete success) + results.push status: !!e.send(method), event: e # rubocop:disable Style/DoubleNegation end results end diff --git a/db/migrate/20230119143245_add_deleted_at_to_event.rb b/db/migrate/20230119143245_add_deleted_at_to_event.rb new file mode 100644 index 000000000..a9ef9f7da --- /dev/null +++ b/db/migrate/20230119143245_add_deleted_at_to_event.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +# Allow soft destroy of events. +# Events with existing reservation cannot be destroyed because we need them for rebuilding invoices, statistics, etc. +# This attribute allows to make a "soft destroy" of an Event, marking it as destroyed so it doesn't appear anymore in +# the interface (as if it was destroyed) but still lives in the database so we can use it to build data. +class AddDeletedAtToEvent < ActiveRecord::Migration[5.2] + def change + add_column :events, :deleted_at, :datetime + add_index :events, :deleted_at + end +end diff --git a/db/schema.rb b/db/schema.rb index a7fb8886f..e4c6f03fc 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: 2023_01_06_081943) do +ActiveRecord::Schema.define(version: 2023_01_19_143245) do # These are extensions that must be enabled in order to support this database enable_extension "fuzzystrmatch" @@ -19,8 +19,8 @@ ActiveRecord::Schema.define(version: 2023_01_06_081943) do enable_extension "unaccent" create_table "abuses", id: :serial, force: :cascade do |t| - t.integer "signaled_id" t.string "signaled_type" + t.integer "signaled_id" t.string "first_name" t.string "last_name" t.string "email" @@ -68,8 +68,8 @@ ActiveRecord::Schema.define(version: 2023_01_06_081943) do t.string "locality" t.string "country" t.string "postal_code" - t.integer "placeable_id" t.string "placeable_type" + t.integer "placeable_id" t.datetime "created_at" t.datetime "updated_at" end @@ -93,8 +93,8 @@ ActiveRecord::Schema.define(version: 2023_01_06_081943) do end create_table "assets", id: :serial, force: :cascade do |t| - t.integer "viewable_id" t.string "viewable_type" + t.integer "viewable_id" t.string "attachment" t.string "type" t.datetime "created_at" @@ -176,8 +176,8 @@ ActiveRecord::Schema.define(version: 2023_01_06_081943) do end create_table "credits", id: :serial, force: :cascade do |t| - t.integer "creditable_id" t.string "creditable_type" + t.integer "creditable_id" t.integer "plan_id" t.integer "hours" t.datetime "created_at" @@ -226,8 +226,10 @@ ActiveRecord::Schema.define(version: 2023_01_06_081943) do t.integer "recurrence_id" t.integer "age_range_id" t.integer "category_id" + t.datetime "deleted_at" t.index ["availability_id"], name: "index_events_on_availability_id" t.index ["category_id"], name: "index_events_on_category_id" + t.index ["deleted_at"], name: "index_events_on_deleted_at" t.index ["recurrence_id"], name: "index_events_on_recurrence_id" end @@ -417,15 +419,15 @@ ActiveRecord::Schema.define(version: 2023_01_06_081943) do create_table "notifications", id: :serial, force: :cascade do |t| t.integer "receiver_id" - t.integer "attached_object_id" t.string "attached_object_type" + t.integer "attached_object_id" t.integer "notification_type_id" t.boolean "is_read", default: false t.datetime "created_at" t.datetime "updated_at" t.string "receiver_type" t.boolean "is_send", default: false - t.jsonb "meta_data", default: {} + t.jsonb "meta_data", default: "{}" t.index ["notification_type_id"], name: "index_notifications_on_notification_type_id" t.index ["receiver_id"], name: "index_notifications_on_receiver_id" end @@ -665,8 +667,8 @@ ActiveRecord::Schema.define(version: 2023_01_06_081943) do create_table "prices", id: :serial, force: :cascade do |t| t.integer "group_id" t.integer "plan_id" - t.integer "priceable_id" t.string "priceable_type" + t.integer "priceable_id" t.integer "amount" t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -867,8 +869,8 @@ ActiveRecord::Schema.define(version: 2023_01_06_081943) do t.text "message" t.datetime "created_at" t.datetime "updated_at" - t.integer "reservable_id" t.string "reservable_type" + t.integer "reservable_id" t.integer "nb_reserve_places" t.integer "statistic_profile_id" t.index ["reservable_type", "reservable_id"], name: "index_reservations_on_reservable_type_and_reservable_id" @@ -877,8 +879,8 @@ ActiveRecord::Schema.define(version: 2023_01_06_081943) do create_table "roles", id: :serial, force: :cascade do |t| t.string "name" - t.integer "resource_id" t.string "resource_type" + t.integer "resource_id" t.datetime "created_at" t.datetime "updated_at" t.index ["name", "resource_type", "resource_id"], name: "index_roles_on_name_and_resource_type_and_resource_id" diff --git a/test/integration/events/delete_test.rb b/test/integration/events/delete_test.rb new file mode 100644 index 000000000..245bc62e6 --- /dev/null +++ b/test/integration/events/delete_test.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'test_helper' + +module Events; end + +class Events::DeleteTest < ActionDispatch::IntegrationTest + setup do + admin = User.with_role(:admin).first + login_as(admin, scope: :user) + end + + test 'delete an event' do + event = Event.first + delete "/api/events/#{event.id}?mode=single", headers: default_headers + + # Check response format & status + assert_response :success + assert_equal Mime[:json], response.content_type + res = json_response(response.body) + assert_equal 1, res[:deleted] + + # Check the event was correctly deleted + assert_raise ActiveRecord::RecordNotFound do + event.reload + end + end + + test 'soft delete an event' do + event = Event.first + + # Make a reservation on this event + post '/api/local_payment/confirm_payment', + params: { + customer_id: User.find_by(username: 'pdurand').id, + items: [ + { + reservation: { + reservable_id: event.id, + reservable_type: 'Event', + nb_reserve_places: 2, + slots_reservations_attributes: [ + { + slot_id: event.availability.slots.first&.id, + offered: false + } + ] + } + } + ] + }.to_json, + headers: default_headers + + # Check response format & status + assert_equal 201, response.status, response.body + + assert_not event.destroyable? + delete "/api/events/#{event.id}?mode=single", headers: default_headers + assert_response :success + res = json_response(response.body) + assert_equal 1, res[:deleted] + + event.reload + assert_not_nil event.deleted_at + end +end diff --git a/test/integration/machines_test.rb b/test/integration/machines_test.rb index 8bbb97f53..ea7d5ba62 100644 --- a/test/integration/machines_test.rb +++ b/test/integration/machines_test.rb @@ -69,9 +69,13 @@ class MachinesTest < ActionDispatch::IntegrationTest end test 'delete a machine' do - delete '/api/machines/3', headers: default_headers + machine = Machine.find(3) + delete "/api/machines/#{machine.id}", headers: default_headers assert_response :success assert_empty response.body + assert_raise ActiveRecord::RecordNotFound do + machine.reload + end end test 'soft delete a machine' do