diff --git a/Gemfile b/Gemfile index bed7440c4..cb5179ad2 100644 --- a/Gemfile +++ b/Gemfile @@ -68,6 +68,7 @@ gem 'public_activity' group :development do gem 'better_errors' + gem 'binding_of_caller' gem 'letter_opener' gem 'web-console', '>= 4.1.0' # Display performance information such as SQL time and flame graphs for each request in your browser. diff --git a/Gemfile.lock b/Gemfile.lock index 1a5a3e015..656078520 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -88,6 +88,8 @@ GEM rack (>= 0.9.0) rouge (>= 1.0.0) bindex (0.8.1) + binding_of_caller (1.0.0) + debug_inspector (>= 0.0.1) bootsnap (1.17.0) msgpack (~> 1.2) bootstrap (5.3.2) @@ -139,6 +141,7 @@ GEM database_cleaner-core (~> 2.0.0) database_cleaner-core (2.0.1) date (3.3.3) + debug_inspector (1.2.0) delayed_job (4.1.11) activesupport (>= 3.0, < 8.0) delayed_job_active_record (4.1.8) @@ -488,6 +491,7 @@ PLATFORMS DEPENDENCIES acts-as-taggable-on better_errors + binding_of_caller bootsnap bootstrap (~> 5) bullet diff --git a/app/controllers/admin/invitations_controller.rb b/app/controllers/admin/invitations_controller.rb index c9d654997..9f49d6040 100644 --- a/app/controllers/admin/invitations_controller.rb +++ b/app/controllers/admin/invitations_controller.rb @@ -39,7 +39,12 @@ def update_to_attended end def update_to_attending - update_successful = @invitation.update(attending: true, rsvp_time: Time.zone.now, automated_rsvp: true) + update_successful = @invitation.update( + attending: true, + rsvp_time: Time.zone.now, + automated_rsvp: true, + last_overridden_by_id: current_user.id + ) { message: update_successful ? attending_successful : attending_failed, @@ -59,7 +64,7 @@ def attending_failed end def update_to_not_attending - @invitation.update(attending: false) + @invitation.update!(attending: false, last_overridden_by_id: current_user.id) { message: "You have removed #{@invitation.member.full_name} from the workshop.", diff --git a/app/controllers/admin/workshops_controller.rb b/app/controllers/admin/workshops_controller.rb index 2afe00080..62694a176 100644 --- a/app/controllers/admin/workshops_controller.rb +++ b/app/controllers/admin/workshops_controller.rb @@ -3,7 +3,7 @@ class Admin::WorkshopsController < Admin::ApplicationController include Admin::WorkshopConcerns before_action :set_workshop_by_id, only: %i[show edit destroy update] - before_action :set_and_decorate_workshop, only: %i[attendees_checklist attendees_emails send_invites] + before_action :set_and_decorate_workshop, only: %i[attendees_checklist attendees_emails send_invites changes] WORKSHOP_DELETION_TIME_FRAME_SINCE_CREATION = 4.hours @@ -105,6 +105,15 @@ def destroy end end + def changes + invitations = @workshop.invitations + .where.not(attending: nil) + .includes(:member).order('members.name') + + @coach_invitations = invitations.to_coaches + @student_invitations = invitations.to_students + end + private def workshop_params diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index af8ca0f8e..fc90f2386 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -4,12 +4,15 @@ class ApplicationController < ActionController::Base include Pundit::Authorization include Pagy::Backend - rescue_from Exception do |ex| - Rollbar.error(ex) - Rails.logger.fatal(ex) - respond_to do |format| - format.html { render 'errors/error', layout: false, status: :internal_server_error } - format.all { head :internal_server_error } + if Rails.env.production? + rescue_from Exception do |ex| + Rollbar.error(ex) + Rails.logger.fatal(ex) + + respond_to do |format| + format.html { render 'errors/error', layout: false, status: :internal_server_error } + format.all { head :internal_server_error } + end end end diff --git a/app/models/member.rb b/app/models/member.rb index 0e3db5194..d13eac0ec 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -48,9 +48,13 @@ def banned_permanently? bans.permanent.present? end + def name_and_surname + [name, surname].compact.join ' ' + end + def full_name pronoun = pronouns.present? ? "(#{pronouns})" : nil - [name, surname, pronoun].compact.join ' ' + [name_and_surname, pronoun].compact.join ' ' end def student? diff --git a/app/models/workshop_invitation.rb b/app/models/workshop_invitation.rb index 8f9d56050..ecd4b560e 100644 --- a/app/models/workshop_invitation.rb +++ b/app/models/workshop_invitation.rb @@ -2,12 +2,16 @@ class WorkshopInvitation < ApplicationRecord include InvitationConcerns belongs_to :workshop + belongs_to :member + belongs_to :overrider, foreign_key: :last_overridden_by_id, class_name: 'Member', inverse_of: false has_one :waiting_list, foreign_key: :invitation_id validates :workshop, :member, presence: true validates :member_id, uniqueness: { scope: %i[workshop_id role] } validates :role, inclusion: { in: %w[Student Coach], allow_nil: true } - validates :tutorial, presence: true, if: :student_attending? + validates :tutorial, presence: true, if: lambda { + student_attending? && !automated_rsvp + } validates :tutorial, presence: true, on: :waitinglist, if: :student_attending? scope :year, ->(year) { joins(:workshop).where('EXTRACT(year FROM workshops.date_and_time) = ?', year) } @@ -39,4 +43,8 @@ def parent def student_attending? for_student? && attending.present? end + + def not_attending? + attending == false + end end diff --git a/app/views/admin/workshop/_attendances.html.haml b/app/views/admin/workshop/_attendances.html.haml index a9d9e489a..d0800f1a8 100644 --- a/app/views/admin/workshop/_attendances.html.haml +++ b/app/views/admin/workshop/_attendances.html.haml @@ -42,9 +42,15 @@ %i.fas.fa-history = l(invitation.rsvp_time) - if invitation.automated_rsvp? - %span{'data-bs-toggle': 'tooltip', 'data-bs-placement': 'bottom', title: 'Waiting list or admin addition'} - %p.mb-1.small - %i.fas.fa-magic + - tooltip_args = { 'data-bs-toggle': 'tooltip', 'data-bs-placement': 'bottom' } + - if invitation.last_overridden_by_id? + %span{ tooltip_args, title: "Addition by #{invitation.overrider.full_name}" } + %p.mb-1.small + %i.fas.fa-hat-wizard + - else + %span{ tooltip_args, title: 'Waiting list or admin addition' } + %p.mb-1.small + %i.fas.fa-magic - if invitation.reminded_at.present? %span{'data-bs-toggle': 'tooltip', 'data-bs-placement': 'bottom', title: 'Reminder emailed at'} %p.mb-1.small diff --git a/app/views/admin/workshops/_activity.html.haml b/app/views/admin/workshops/_activity.html.haml new file mode 100644 index 000000000..5a722f511 --- /dev/null +++ b/app/views/admin/workshops/_activity.html.haml @@ -0,0 +1,15 @@ +%tr + %th{ scope: "col" } + = link_to invitation.member.name_and_surname, + admin_member_path(invitation.member_id) + %td + - if invitation.not_attending? + %span.text-muted No + - else + Yes + %td + - if invitation.last_overridden_by_id? + = link_to invitation.overrider.full_name, + admin_member_path(invitation.last_overridden_by_id) + - else + %span.text-muted No diff --git a/app/views/admin/workshops/_invitation_management.html.haml b/app/views/admin/workshops/_invitation_management.html.haml index a2f8b4a59..af2985322 100644 --- a/app/views/admin/workshops/_invitation_management.html.haml +++ b/app/views/admin/workshops/_invitation_management.html.haml @@ -9,6 +9,8 @@ #{@attending_students.count} are attending as students and #{@attending_coaches.count} as coaches. - if @student_waiting_list.any? or @coach_waiting_list.any? There is also a waiting list of #{@student_waiting_list.count} students and #{@coach_waiting_list.count} coaches. + %br + = link_to 'See all invitations’ statuses', admin_workshop_changes_path(@workshop) = simple_form_for :workshop, url: admin_workshop_invitations_path(@workshop, attending: true), remote: true, method: :put do |f| .row.mb-4 diff --git a/app/views/admin/workshops/changes.html.haml b/app/views/admin/workshops/changes.html.haml new file mode 100644 index 000000000..71358f0e3 --- /dev/null +++ b/app/views/admin/workshops/changes.html.haml @@ -0,0 +1,30 @@ +.container + %p + This page is useful to see all the invitations acted upon, meaning any people + that RSVPed attending or not attending. + %p + You can also see if an invitation has been overridden by an organiser, + usually to manually add/remove people from workshops. + + %table.table.table-striped.students-table + %thead + %tr + %th{ scope: "col" } Student + %th{ scope: "col" } Attending? + %th{ scope: "col" } Overriden by organiser? + %tbody + - @student_invitations.each do |invitation| + = render partial: 'admin/workshops/activity', + locals: { invitation: invitation } + +.container + %table.table.table-striped.coaches-table + %thead + %tr + %th{ scope: "col" } Coach + %th{ scope: "col" } Attending? + %th{ scope: "col" } Overriden by organiser? + %tbody + - @coach_invitations.each do |invitation| + = render partial: 'admin/workshops/activity', + locals: { invitation: invitation } diff --git a/app/views/invitations/index.html.haml b/app/views/invitations/index.html.haml index ac81e8f46..c63550fdf 100644 --- a/app/views/invitations/index.html.haml +++ b/app/views/invitations/index.html.haml @@ -23,8 +23,9 @@ Group: #{invitation.role.pluralize} %br .text-muted #{humanize_date(invitation.parent.date_and_time, with_time: true)} - - if invitation.is_a? WorkshopInvitation - = link_to attendance_status(invitation), invitation_path(invitation), class: 'btn btn-sm btn-primary', role: 'button' + - if invitation.is_a? InvitationPresenter + = link_to invitation.attendance_status, invitation_path(invitation), + class: 'btn btn-sm btn-primary', role: 'button' - elsif @upcoming_workshop %p You have no invitations. If you just signed up you should receive one soon. diff --git a/config/routes.rb b/config/routes.rb index df600ea99..4b0c19996 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -133,6 +133,7 @@ get 'attendees_checklist' get 'attendees_emails' get 'send_invites' + get 'changes' resource :invitations, only: [:update] resources :invitations, only: [:update] diff --git a/db/migrate/20231230162506_add_last_overridden_by_id_to_workshop_invitations.rb b/db/migrate/20231230162506_add_last_overridden_by_id_to_workshop_invitations.rb new file mode 100644 index 000000000..0d99c35d5 --- /dev/null +++ b/db/migrate/20231230162506_add_last_overridden_by_id_to_workshop_invitations.rb @@ -0,0 +1,5 @@ +class AddLastOverriddenByIdToWorkshopInvitations < ActiveRecord::Migration[7.0] + def change + add_column :workshop_invitations, :last_overridden_by_id, :integer, default: nil + end +end diff --git a/db/schema.rb b/db/schema.rb index 750569f9b..623b6f85e 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[7.0].define(version: 2023_08_03_225601) do +ActiveRecord::Schema[7.0].define(version: 2023_12_30_162506) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -526,6 +526,7 @@ t.datetime "rsvp_time", precision: nil t.boolean "automated_rsvp" t.text "tutorial" + t.integer "last_overridden_by_id" t.index ["member_id"], name: "index_workshop_invitations_on_member_id" t.index ["token"], name: "index_workshop_invitations_on_token", unique: true t.index ["workshop_id"], name: "index_workshop_invitations_on_workshop_id" diff --git a/spec/controllers/admin/invitations_controller_spec.rb b/spec/controllers/admin/invitations_controller_spec.rb index f56cde0b1..f7d6b9d66 100644 --- a/spec/controllers/admin/invitations_controller_spec.rb +++ b/spec/controllers/admin/invitations_controller_spec.rb @@ -22,15 +22,27 @@ expect(flash[:notice]).to match("You have added") end - it "Warns the user about failed updates" do - # Trigger an error when trying to update the `attending` attribute + # While similar to the previous test, this specifically tests that organisers + # have the ability to manually add a student to the workshop that has not + # selected a tutorial. This is helpful for when a student shows up for a + # workshop they have not have a spot — this happens from time to time. + it "Successfuly adds a user as attenting, even without a tutorial" do invitation.update_attribute(:tutorial, nil) + expect(invitation.automated_rsvp).to be_nil put :update, params: { id: invitation.token, workshop_id: workshop.id, attending: "true" } + invitation.reload + + expect(invitation.attending).to be true + expect(invitation.automated_rsvp).to be true + expect(flash[:notice]).to match("You have added") + end + + it "Records the organiser ID that overrides an invitations" do + put :update, params: { id: invitation.token, workshop_id: workshop.id, attending: "true" } + invitation.reload - # State didn't change and we have an error message explaining why - expect(invitation.reload.attending).to be_nil - expect(flash[:notice]).to match("Tutorial must be selected.") + expect(invitation.last_overridden_by_id).to be admin.id end end end diff --git a/spec/features/admin/manage_workshop_attendances_spec.rb b/spec/features/admin/manage_workshop_attendances_spec.rb index 6c5b05b42..2571f8813 100644 --- a/spec/features/admin/manage_workshop_attendances_spec.rb +++ b/spec/features/admin/manage_workshop_attendances_spec.rb @@ -38,7 +38,7 @@ expect(page).to have_content('2 are attending as students') expect(page).to have_content(I18n.l(other_invitation.reload.rsvp_time)) - expect(page).to have_selector('i.fa-magic') + expect(page).to have_selector('i.fa-hat-wizard') end scenario 'can rsvp an invited student to the workshop', js: true do @@ -56,7 +56,7 @@ expect(page).to have_content('2 are attending as students') expect(page).to have_content(I18n.l(other_invitation.reload.rsvp_time)) - expect(page).to have_css('.fa-magic') + expect(page).to have_css('.fa-hat-wizard') end scenario 'can view the tutorial and note set by an attendee' do @@ -67,5 +67,38 @@ expect(page).to have_content(invitation.note) expect(page).to have_content(invitation.tutorial) end + + context '#changes' do + before do + # Workshop invitations without `attending` status + Fabricate(:workshop_invitation, workshop: workshop, role: 'Coach') + Fabricate(:workshop_invitation, workshop: workshop, role: 'Student') + + # Not attending + Fabricate(:workshop_invitation, workshop: workshop, role: 'Coach', attending: false) + Fabricate(:workshop_invitation, workshop: workshop, role: 'Student', attending: false) + + # Attending, with a student having been manually added/confirmed by an organiser + Fabricate(:attending_workshop_invitation, workshop: workshop, role: 'Coach') + Fabricate(:attending_workshop_invitation, workshop: workshop, role: 'Student') + overridden = Fabricate(:attending_workshop_invitation, workshop: workshop, role: 'Student') + overridden.update(last_overridden_by_id: member.id) + end + + scenario 'can verify if a invitation has been overridden by an organiser' do + visit admin_workshop_changes_path(workshop) + + expect(page).to have_css( + '.coaches-table tbody tr', + count: workshop.invitations.to_coaches.where.not(attending: nil).count + ) + expect(page).to have_css( + '.students-table tbody tr', + count: workshop.invitations.to_students.where.not(attending: nil).count + ) + + expect(page).to have_link(member.name_and_surname, href: admin_member_path(member.id)) + end + end end end