From 4e15a42212bc4e344a5ba2c6e1797632158cbd43 Mon Sep 17 00:00:00 2001 From: Drew Teter Date: Mon, 15 Aug 2022 00:06:36 -0400 Subject: [PATCH] OCC-859 OCC-860 OCC-865 OCC-866 --- .../stylesheets/admin/_general-styles.scss | 1 + app/assets/stylesheets/admin/_tables.scss | 4 -- .../purposes_travel_patterns_controller.rb | 19 ++++++--- .../admin/service_schedules_controller.rb | 41 ++++++++++--------- app/models/funding_source.rb | 1 + app/models/purpose.rb | 2 + app/models/service_schedule.rb | 3 +- app/models/service_sub_schedule.rb | 2 +- .../admin/booking_windows/index.html.haml | 10 ++--- .../admin/funding_sources/index.html.haml | 10 ++--- .../landmark_sets/_selected_pois.html.haml | 2 +- .../landmark_sets/_system_pois.html.haml | 2 +- app/views/admin/landmark_sets/index.html.haml | 13 +++--- app/views/admin/od_zones/_od_zone.html.haml | 8 ++-- .../purposes_travel_patterns/index.html.haml | 10 ++--- .../_service_schedule.html.haml | 8 ++-- .../travel_patterns/_travel_pattern.html.haml | 8 ++-- spec/factories/funding_sources.rb | 2 +- spec/factories/purposes.rb | 6 +-- 19 files changed, 79 insertions(+), 73 deletions(-) diff --git a/app/assets/stylesheets/admin/_general-styles.scss b/app/assets/stylesheets/admin/_general-styles.scss index ba353e0c8..226157dc8 100644 --- a/app/assets/stylesheets/admin/_general-styles.scss +++ b/app/assets/stylesheets/admin/_general-styles.scss @@ -63,6 +63,7 @@ footer.footer__links { .btn.btn--no-bg { border: none; background:transparent; + line-height: 1; } .flex-row { diff --git a/app/assets/stylesheets/admin/_tables.scss b/app/assets/stylesheets/admin/_tables.scss index 760b544e6..d707a24d4 100644 --- a/app/assets/stylesheets/admin/_tables.scss +++ b/app/assets/stylesheets/admin/_tables.scss @@ -1,7 +1,3 @@ -.td-buttons { - max-width: 80px; -} - .highlight { background-color: lightblue; } \ No newline at end of file diff --git a/app/controllers/admin/purposes_travel_patterns_controller.rb b/app/controllers/admin/purposes_travel_patterns_controller.rb index a0ed97a3d..aa3b58138 100644 --- a/app/controllers/admin/purposes_travel_patterns_controller.rb +++ b/app/controllers/admin/purposes_travel_patterns_controller.rb @@ -1,5 +1,5 @@ class Admin::PurposesTravelPatternsController < Admin::AdminController - before_action :load_agency_from_params_or_user, only: [:new] + before_action :load_agency_from_params_or_user, only: [:new, :create] def index @purposes = Purpose.accessible_by(current_ability) @@ -31,9 +31,12 @@ def create @purpose = Purpose.new(purpose_params) @purpose.agency_id = params[:agency_id] authorize! :create, @purpose - @purpose.save - - redirect_to admin_trip_purposes_path + if @purpose.save + redirect_to admin_trip_purposes_path + else + flash.now[:danger] = 'Trip Purpose could not be created.' + render :new + end end def edit @@ -44,8 +47,12 @@ def edit def update @purpose = Purpose.find(params[:id]) authorize! :edit, @purpose - @purpose.update(purpose_params) - redirect_to admin_trip_purposes_path + if @purpose.update(purpose_params) + redirect_to admin_trip_purposes_path + else + flash.now[:danger] = 'Trip Purpose could not be updated.' + render :edit + end end private diff --git a/app/controllers/admin/service_schedules_controller.rb b/app/controllers/admin/service_schedules_controller.rb index e1ea5f11c..2b80215dd 100644 --- a/app/controllers/admin/service_schedules_controller.rb +++ b/app/controllers/admin/service_schedules_controller.rb @@ -23,6 +23,7 @@ def new end def create + debugger service_schedule_params = params.require(:service_schedule).except(:service_sub_schedules_attributes, :sub_schedule_calendar_dates_attributes, :sub_schedule_calendar_times_attributes).permit! if params[:service_schedule][:service_sub_schedules_attributes] sub_schedule_params = params.require(:service_schedule).require(:service_sub_schedules_attributes) @@ -42,9 +43,10 @@ def create error_message = @service_schedule.errors.full_messages.join("\n") raise ActiveRecord::Rollback end + if service_schedule_params[:start_date] && service_schedule_params[:end_date] @service_schedule.start_date = service_schedule_params[:start_date].blank? ? nil : Date.parse(service_schedule_params[:start_date], "%Y/%m/%d") - @service_schedule.end_date = service_schedule_params[:end_date].blank? ? nil : Date.parse(service_schedule_params[:start_date], "%Y/%m/%d") + @service_schedule.end_date = service_schedule_params[:end_date].blank? ? nil : Date.parse(service_schedule_params[:end_date], "%Y/%m/%d") end if @service_schedule.save @@ -59,6 +61,7 @@ def create sub_schedule.service_schedule = @service_schedule unless sub_schedule.save! schedule_created = false + debugger error_message = sub_schedule.errors.full_messages.join("\n") raise ActiveRecord::Rollback end @@ -161,33 +164,31 @@ def update # Editing Weekly pattern schedule type if @service_schedule.is_a_weekly_schedule? && sub_schedule_params sub_schedule_params.each do |s| - if s[:_destroy] == "true" - unless s[:id].blank? - unless deleted_schedule = ServiceSubSchedule.find_by(id: s[:id]).destroy - error_message = deleted_schedule.errors.full_messages.join("\n") - schedule_updated = false - raise ActiveRecord::Rollback - end - schedule_updated = true + if s[:id].blank? + sub_schedule = ServiceSubSchedule.new(s.except(:_destroy).permit!) + sub_schedule.service_schedule = @service_schedule + unless sub_schedule.save! + schedule_updated = falsequi + error_message = sub_schedule.errors.full_messages.join("\n") + raise ActiveRecord::Rollback end + schedule_updated = true else - if s[:id].blank? - sub_schedule = ServiceSubSchedule.new(s.except(:_destroy).permit!) - sub_schedule.service_schedule = @service_schedule - unless sub_schedule.save! - schedule_updated = false + sub_schedule = ServiceSubSchedule.find_by(id: s[:id]) + if s[:_destroy] == "true" + unless sub_schedule.destroy error_message = sub_schedule.errors.full_messages.join("\n") - raise ActiveRecord::Rollback - end - schedule_updated = true - else - unless updated_schedule = ServiceSubSchedule.find_by(id: s[:id]).update(s.except(:_destroy).permit!) schedule_updated = false - error_message = updated_schedule.errors.full_messages.join("\n") raise ActiveRecord::Rollback end schedule_updated = true + elsif !sub_schedule.update(s.except(:_destroy).permit!) + # debugger + schedule_updated = false + error_message = sub_schedule.errors.full_messages.join("\n") + raise ActiveRecord::Rollback end + schedule_updated = true end end diff --git a/app/models/funding_source.rb b/app/models/funding_source.rb index 68a9638d8..1e6b1c306 100644 --- a/app/models/funding_source.rb +++ b/app/models/funding_source.rb @@ -2,4 +2,5 @@ class FundingSource < ApplicationRecord belongs_to :agency validates_presence_of :name, :description, :agency + validates :name, uniqueness: {scope: :agency_id} end diff --git a/app/models/purpose.rb b/app/models/purpose.rb index 9b7832b36..736abd96d 100644 --- a/app/models/purpose.rb +++ b/app/models/purpose.rb @@ -10,6 +10,7 @@ class Purpose < ApplicationRecord before_save :snake_casify, if: :has_code? validate :name_is_present? + validates :name, uniqueness: {scope: :agency_id} def has_code? code.present? @@ -17,5 +18,6 @@ def has_code? def name_is_present? errors.add(:name, :blank) if self[:name].blank? + errors.add(:name, :taken) if Purpose.where.not(id: id).exists?(name: self[:name], agency_id: agency_id) end end diff --git a/app/models/service_schedule.rb b/app/models/service_schedule.rb index 6ac1f1a5d..cef7ec2f3 100644 --- a/app/models/service_schedule.rb +++ b/app/models/service_schedule.rb @@ -15,13 +15,14 @@ class ServiceSchedule < ApplicationRecord belongs_to :agency belongs_to :service_schedule_type has_many :service_sub_schedules, dependent: :destroy - has_many :travel_pattern_service_schedules + has_many :travel_pattern_service_schedules, dependent: :destroy has_many :travel_patterns, through: :travel_pattern_service_schedules attr_accessor :sub_schedule_calendar_dates attr_accessor :sub_schedule_calendar_times accepts_nested_attributes_for :service_sub_schedules + validates :service_schedule_type, presence: true validates :name, presence: true, uniqueness: {scope: :agency_id} validate :end_date_after_start_date diff --git a/app/models/service_sub_schedule.rb b/app/models/service_sub_schedule.rb index 4bf9c5438..36aef2bf4 100644 --- a/app/models/service_sub_schedule.rb +++ b/app/models/service_sub_schedule.rb @@ -110,7 +110,7 @@ def attributes_for_midnight_shim # Validates that start_time is at or before end_time def start_time_must_be_before_end_time - errors.add(:start_time, "start time cannot be after end time") if (start_time > end_time) + errors.add(:start_time, "cannot be after end time") if (start_time > end_time) end end diff --git a/app/views/admin/booking_windows/index.html.haml b/app/views/admin/booking_windows/index.html.haml index 3734e8cea..a4a44e146 100644 --- a/app/views/admin/booking_windows/index.html.haml +++ b/app/views/admin/booking_windows/index.html.haml @@ -16,14 +16,14 @@ =booking_window.name %td.text-right - if can? :update, BookingWindow - =link_to edit_admin_booking_window_path(booking_window) do - %span.glyphicon.glyphicon-edit + =link_to edit_admin_booking_window_path(booking_window), class: "btn btn--no-bg" do + %span.glyphicon.glyphicon-pencil - else - =link_to admin_booking_window_path(booking_window) do + =link_to admin_booking_window_path(booking_window), class: "btn btn--no-bg" do %span.glyphicon.glyphicon-eye-open - if can? :delete, BookingWindow - =link_to admin_booking_window_path(booking_window), method: :delete, data: {confirm: 'Warning: This will delete the Booking Window and it will be removed from any travel patterns in which it is currently being used. Press "OK" to confirm.'} do - %span.glyphicon.glyphicon-trash{ style:'color:red' } + =link_to admin_booking_window_path(booking_window), class: "btn text-danger btn--no-bg", method: :delete, data: {confirm: 'Warning: This will delete the Booking Window and it will be removed from any travel patterns in which it is currently being used. Press "OK" to confirm.'} do + %span.glyphicon.glyphicon-trash %div.text-center - modalID = 'new-booking-window' diff --git a/app/views/admin/funding_sources/index.html.haml b/app/views/admin/funding_sources/index.html.haml index 1cb6ee8f0..3233af7c7 100644 --- a/app/views/admin/funding_sources/index.html.haml +++ b/app/views/admin/funding_sources/index.html.haml @@ -16,14 +16,14 @@ =funding_source.name %td.text-right - if can? :update, FundingSource - =link_to edit_admin_funding_source_path(funding_source) do - %span.glyphicon.glyphicon-edit + =link_to edit_admin_funding_source_path(funding_source), class: "btn btn--no-bg" do + %span.glyphicon.glyphicon-pencil - else - =link_to admin_funding_source_path(funding_source) do + =link_to admin_funding_source_path(funding_source), class: "btn btn--no-bg" do %span.glyphicon.glyphicon-eye-open - if can? :delete, FundingSource - =link_to admin_funding_source_path(funding_source), method: :delete, data: {confirm: 'Warning: This will delete the funding source and it will be removed from any travel patterns in which it is currently being used. Press "OK" to confirm.'} do - %span.glyphicon.glyphicon-trash{ style:'color:red' } + =link_to admin_funding_source_path(funding_source), class: "btn text-danger btn--no-bg", method: :delete, data: {confirm: 'Warning: This will delete the funding source and it will be removed from any travel patterns in which it is currently being used. Press "OK" to confirm.'} do + %span.glyphicon.glyphicon-trash %div.text-center - modalID = 'new-funding-source' diff --git a/app/views/admin/landmark_sets/_selected_pois.html.haml b/app/views/admin/landmark_sets/_selected_pois.html.haml index 388ce59c2..411e011ea 100644 --- a/app/views/admin/landmark_sets/_selected_pois.html.haml +++ b/app/views/admin/landmark_sets/_selected_pois.html.haml @@ -11,7 +11,7 @@ %thead.thead-default %tr %th - %th.td-buttons{'aria-label':'Buttons'} + %th.text-right{'aria-label':'Buttons'} %tbody -if @selected_pois.length > 0 -@selected_pois.each do |poi| diff --git a/app/views/admin/landmark_sets/_system_pois.html.haml b/app/views/admin/landmark_sets/_system_pois.html.haml index 4dda4cc2b..2dd61f64c 100644 --- a/app/views/admin/landmark_sets/_system_pois.html.haml +++ b/app/views/admin/landmark_sets/_system_pois.html.haml @@ -11,7 +11,7 @@ %thead.thead-default %tr %th - %th.td-buttons{'aria-label':'Buttons'} + %th{'aria-label':'Buttons'} %tbody -if @system_pois.length > 0 -@system_pois.each do |poi| diff --git a/app/views/admin/landmark_sets/index.html.haml b/app/views/admin/landmark_sets/index.html.haml index e9f504b06..b53ff7fce 100644 --- a/app/views/admin/landmark_sets/index.html.haml +++ b/app/views/admin/landmark_sets/index.html.haml @@ -6,24 +6,21 @@ %tr %th Agency %th Name - %th.td-buttons{'aria-label':'Buttons'} + %th{'aria-label':'Buttons'} -@landmark_sets.each do |sets| %tr{style: "cursor: pointer", data: {url: admin_landmark_sets_path}} %td =sets.agency&.name %td =sets.name - %td.td-buttons + %td.text-right -if can?(:show, LandmarkSet) && cannot?(:edit, LandmarkSet) - =link_to admin_landmark_sets_path, - class: "btn btn--no-bg" do + =link_to admin_landmark_sets_path, class: "btn btn--no-bg" do %span.glyphicon.glyphicon-eye-open -if can?(:show, LandmarkSet) && can?(:edit, LandmarkSet) - =link_to edit_admin_landmark_set_path(id: sets.id), - class: "btn btn--no-bg" do + =link_to edit_admin_landmark_set_path(id: sets.id), class: "btn btn--no-bg" do %span.glyphicon.glyphicon-pencil{aria:{hidden:true}} - =link_to admin_landmark_set_path(id: sets.id), method: :delete, - class: "btn text-danger btn--no-bg #{can?(:delete, LandmarkSet) ? '': 'disabled'}" do + =link_to admin_landmark_set_path(id: sets.id), method: :delete, class: "btn text-danger btn--no-bg #{can?(:delete, LandmarkSet) ? '': 'disabled'}" do %span.glyphicon.glyphicon-trash{aria:{hidden:true}} %footer.footer__links diff --git a/app/views/admin/od_zones/_od_zone.html.haml b/app/views/admin/od_zones/_od_zone.html.haml index ba15f8eda..8551ea070 100644 --- a/app/views/admin/od_zones/_od_zone.html.haml +++ b/app/views/admin/od_zones/_od_zone.html.haml @@ -3,13 +3,13 @@ =od_zone&.agency || Agency.first %td =od_zone.name - %td + %td.text-right %div - if current_user.superuser? || current_user.oversight_admin? || current_user.transportation_admin? - =link_to edit_admin_od_zone_path(id: od_zone.id), class: "edit_od_zone_action_button action_button", title: "Edit O/D Zone" do + =link_to edit_admin_od_zone_path(id: od_zone.id), class: "btn btn--no-bg", title: "Edit O/D Zone" do %span.glyphicon.glyphicon-pencil - =link_to admin_od_zone_path(od_zone), method: :delete, class: "delete_od_zone_action_button action_button", title: "Delete O/D Zone" do + =link_to admin_od_zone_path(od_zone), method: :delete, class: "btn text-danger btn--no-bg", title: "Delete O/D Zone" do %span.glyphicon.glyphicon-trash - elsif current_user.oversight_staff? || current_user.transportation_staff? - =link_to admin_od_zone_path(od_zone), class: "view_od_zone_action_button action_button", title: "View O/D Zone" do + =link_to admin_od_zone_path(od_zone), class: "btn btn--no-bg", title: "View O/D Zone" do %span.glyphicon.glyphicon-eye-open \ No newline at end of file diff --git a/app/views/admin/purposes_travel_patterns/index.html.haml b/app/views/admin/purposes_travel_patterns/index.html.haml index 9ca46b07c..b706c33e1 100644 --- a/app/views/admin/purposes_travel_patterns/index.html.haml +++ b/app/views/admin/purposes_travel_patterns/index.html.haml @@ -16,14 +16,14 @@ =purpose[:name] %td.text-right - if can? :update, Purpose - =link_to edit_admin_trip_purpose_path(purpose) do - %span.glyphicon.glyphicon-edit + =link_to edit_admin_trip_purpose_path(purpose), class: "btn btn--no-bg" do + %span.glyphicon.glyphicon-pencil - else - =link_to admin_trip_purpose_path(purpose) do + =link_to admin_trip_purpose_path(purpose), class: "btn btn--no-bg" do %span.glyphicon.glyphicon-eye-open - if can? :delete, Purpose - =link_to admin_trip_purpose_path(purpose), method: :delete, data: {confirm: 'Warning: This will delete the trip purpose and it will be removed from any travel patterns in which it is currently being used. Press "OK" to confirm.'} do - %span.glyphicon.glyphicon-trash{ style:'color:red' } + =link_to admin_trip_purpose_path(purpose), class: "btn text-danger btn--no-bg", method: :delete, data: {confirm: 'Warning: This will delete the trip purpose and it will be removed from any travel patterns in which it is currently being used. Press "OK" to confirm.'} do + %span.glyphicon.glyphicon-trash %div.text-center - modalID = 'new-trip-purpose' diff --git a/app/views/admin/service_schedules/_service_schedule.html.haml b/app/views/admin/service_schedules/_service_schedule.html.haml index 07c743082..1c0bb2b15 100644 --- a/app/views/admin/service_schedules/_service_schedule.html.haml +++ b/app/views/admin/service_schedules/_service_schedule.html.haml @@ -9,13 +9,13 @@ =service_schedule.try(:start_date) %td =service_schedule.try(:end_date) - %td + %td.text-right %div - if current_user.superuser? || current_user.oversight_admin? || current_user.transportation_admin? - =link_to edit_admin_service_schedule_path(id: service_schedule.id), class: "edit_service_schedule_action_button action_button", title: "Edit Service Schedule" do + =link_to edit_admin_service_schedule_path(id: service_schedule.id), class: "btn btn--no-bg", title: "Edit Service Schedule" do %span.glyphicon.glyphicon-pencil - =link_to admin_service_schedule_path(service_schedule), method: :delete, class: "delete_service_schedule_action_button action_button", title: "Delete Service Schedule", data: {confirm: 'Warning: This will delete the service schedule and it will be removed from any travel patterns in which it is currently being used. Press "OK" to confirm.'} do + =link_to admin_service_schedule_path(service_schedule), method: :delete, class: "btn text-danger btn--no-bg", title: "Delete Service Schedule", data: {confirm: 'Warning: This will delete the service schedule and it will be removed from any travel patterns in which it is currently being used. Press "OK" to confirm.'} do %span.glyphicon.glyphicon-trash - elsif current_user.oversight_staff? || current_user.transportation_staff? - =link_to admin_service_schedule_path(service_schedule), class: "view_service_schedule_action_button action_button", title: "View Service Schedule" do + =link_to admin_service_schedule_path(service_schedule), class: "btn btn--no-bg", title: "View Service Schedule" do %span.glyphicon.glyphicon-eye-open \ No newline at end of file diff --git a/app/views/admin/travel_patterns/_travel_pattern.html.haml b/app/views/admin/travel_patterns/_travel_pattern.html.haml index d3f618cea..5e5ae9893 100644 --- a/app/views/admin/travel_patterns/_travel_pattern.html.haml +++ b/app/views/admin/travel_patterns/_travel_pattern.html.haml @@ -3,13 +3,13 @@ =travel_pattern.agency %td =travel_pattern.name - %td + %td.text-right %div - if current_user.superuser? || current_user.oversight_admin? || current_user.transportation_admin? - =link_to edit_admin_travel_pattern_path(id: travel_pattern.id), class: "edit_travel_pattern_action_button action_button", title: "Edit Travel Pattern" do + =link_to edit_admin_travel_pattern_path(id: travel_pattern.id), class: "btn btn--no-bg", title: "Edit Travel Pattern" do %span.glyphicon.glyphicon-pencil - =link_to admin_travel_pattern_path(travel_pattern), method: :delete, class: "delete_travel_pattern_action_button action_button", title: "Delete Travel Pattern" do + =link_to admin_travel_pattern_path(travel_pattern), method: :delete, class: "btn text-danger btn--no-bg", title: "Delete Travel Pattern" do %span.glyphicon.glyphicon-trash - elsif current_user.oversight_staff? || current_user.transportation_staff? - =link_to admin_travel_pattern_path(travel_pattern), class: "view_travel_pattern_action_button action_button", title: "View Travel Pattern" do + =link_to admin_travel_pattern_path(travel_pattern), class: "btn btn--no-bg", title: "View Travel Pattern" do %span.glyphicon.glyphicon-eye-open \ No newline at end of file diff --git a/spec/factories/funding_sources.rb b/spec/factories/funding_sources.rb index 1dd5ffc9b..74fd69f9f 100644 --- a/spec/factories/funding_sources.rb +++ b/spec/factories/funding_sources.rb @@ -1,6 +1,6 @@ FactoryBot.define do factory :funding_source do - name "MyString" + sequence(:name) { |n| "Funding Source #{n}" } description "MyString" agency end diff --git a/spec/factories/purposes.rb b/spec/factories/purposes.rb index 0e00508cd..c88f2d0f8 100644 --- a/spec/factories/purposes.rb +++ b/spec/factories/purposes.rb @@ -1,15 +1,15 @@ FactoryBot.define do factory :purpose do - name 'Medical' + sequence(:name) { |n| "medical #{n}" } code 'medical' factory :metallica_concert do - name 'Metallica!' + sequence(:name) { |n| "Metallica! #{n}" } code "metallica_concert" end - initialize_with { Purpose.find_or_create_by(name: name, code: code) } + initialize_with { Purpose.find_or_create_by(code: code) } # Create translations for the purpose trait :with_translations do