Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add foreign key constraint and relationship checks from timecard_entry to eventdate #529

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion app/models/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class Event < ActiveRecord::Base
validates_associated :organization, :emails, :event_roles, :eventdates
validates_format_of :contactemail, :with => Event::EmailRegex, :multiline => true
# validate :eventdate_valid?
validate :textable_social_valid?
validate :textable_social_valid?, :check_eventdates_to_be_destroyed

scope :current_year, -> { where("representative_date >= ? or last_representative_date > ?", Account.magic_date, Account.magic_date) }

Expand Down Expand Up @@ -230,4 +230,17 @@ def textable_social_valid?
errors.add(:textable_social, "Textable must be enabled if social textable is enabled")
end
end

def check_eventdates_to_be_destroyed
# If we are trying to update an event, someone might try to delete an eventdate.
# This is bad if someone has billed for it. There is before_destroy validation,
# and a foreign key check, but because the eventdate is removed from the event,
# it is not automatically validated on submission. This we must manually check
# that we can delete events here.
eventdates.each do |eventdate|
if eventdate.marked_for_destruction? and !eventdate.can_destroy?
errors.add(:eventdates, "Cannot delete an eventdate that has been billed for")
end
end
end
end
13 changes: 13 additions & 0 deletions app/models/eventdate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ class Eventdate < ApplicationRecord
before_validation :prune_roles
after_save :synchronize_representative_dates

before_destroy :check_can_destroy

Event_Span_Days = 2;
Event_Span_Seconds = Event_Span_Days * 24 * 60 * 60;

Expand Down Expand Up @@ -63,6 +65,17 @@ def valid_strike?
((strikedate.to_i - enddate.to_i) < Event_Span_Seconds))
end

def can_destroy?
not self.timecard_entries.any?
end

def check_can_destroy
if not self.can_destroy?
errors.add(:base, "Cannot delete an eventdate that has been billed for")
throw(:abort)
end
end

def has_call?
self.calltype == "literal" or self.calltype == "startdate"
end
Expand Down
4 changes: 2 additions & 2 deletions app/models/timecard_entry.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
class TimecardEntry < ApplicationRecord
belongs_to :member
belongs_to :eventdate
belongs_to :eventdate, required: true
NoRePercussions marked this conversation as resolved.
Show resolved Hide resolved
belongs_to :timecard

extend Enumerize
enumerize :eventpart, in: ["call", "show", "strike"], default: "show"

validates_presence_of :member_id, :eventdate_id, :hours, :eventpart
validates_presence_of :eventdate, :member_id, :hours, :eventpart
validates_numericality_of :hours, :less_than_or_equal_to => 37.5, :greater_than => 0
validates_associated :member
validates_inclusion_of :timecard, :in => ->(t){Timecard.valid_timecards}, :message => 'is not a current timecard', :allow_nil => true
Expand Down
10 changes: 7 additions & 3 deletions app/views/events/_eventdate_fields.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,14 @@
<% if can? :tic, @event %>
<dl>
<dt>Delete?</dt>
<% if @event.new_record? %>
<dd><%= link_to_remove_fields image_tag("cross.png"), f, true %></dd>
<% if f.object.can_destroy? %>
<% if @event.new_record? %>
<dd><%= link_to_remove_fields image_tag("cross.png"), f, true %></dd>
<% else %>
<dd><%= link_to_remove_fields image_tag("cross.png"), f %></dd>
<% end %>
<% else %>
<dd><%= link_to_remove_fields image_tag("cross.png"), f %></dd>
<dd>You can't delete this date, as someone has already billed for it.</dd>
<% end %>
</dl>
<% end %>
Expand Down
10 changes: 10 additions & 0 deletions db/migrate/20231113195629_add_eventdate_fk_to_timecards.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class AddEventdateFkToTimecards < ActiveRecord::Migration[6.1]
def up
change_column :timecard_entries, :eventdate_id, :bigint
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you changing this to a bigint?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The eventdate ID column is a bigint

add_foreign_key :timecard_entries, :eventdates
end
def down
remove_foreign_key :timecard_entries, :eventdates
change_column :timecard_entries, :eventdate_id, :integer
end
end
5 changes: 3 additions & 2 deletions db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.