Merge pull request #255 from camsys/bug_fix

OCC-859 OCC-860 OCC-865 OCC-866
camsys · Aug 15, 2022 · 7c4be28 · 7c4be28
2 parents bf5a4bb + 4e15a42
commit 7c4be28
Show file tree

Hide file tree

Showing 20 changed files with 79 additions and 74 deletions.
diff --git 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
@@ -1,7 +1,3 @@
-.td-buttons {
- max-width: 80px;
-}
-
 .highlight {
   background-color: lightblue;
 }
diff --git 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
@@ -42,9 +42,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 +60,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 +163,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/controllers/api/v2/trips_controller.rb b/app/controllers/api/v2/trips_controller.rb
@@ -43,7 +43,7 @@ def create
         ).where(
           agency: agency
         ).merge(
-          TravelPatternPurpose.where(purpose_id: params[:purpose_id])
+          TravelPatternPurpose.where(purpose_id: trip_params[:purpose_id])
         ).for_date(trip_date)
 
         # Initialize a trip based on the params

diff --git 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
@@ -10,12 +10,14 @@ 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?
   end
 
   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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git 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
@@ -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
diff --git 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
diff --git 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
@@ -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