Add foreign key constraint and relationship checks from timecard_entry to eventdate #529
Conversation
…y to eventdate Prevent inconsistency when eventdates are deleted by enforcing that timecard entries must reference a valid eventdate
| @@ -0,0 +1,10 @@ | |||
| class AddEventdateFkToTimecards < ActiveRecord::Migration[6.1] | |||
| def up | |||
| change_column :timecard_entries, :eventdate_id, :bigint | |||
There was a problem hiding this comment.
Why are you changing this to a bigint?
There was a problem hiding this comment.
The eventdate ID column is a bigint
app/models/timecard_entry.rb
Outdated
| 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, :eventdate_id, :hours, :eventpart |
There was a problem hiding this comment.
I think you should remove :eventdate_id if adding :eventdate.
NoRePercussions
force-pushed
the
bugs/timecard-referential-integrity
branch
from
fcc589c
to
7c8f252
Compare
Add can_destroy? helper method, orm-level checks before the eventdate is destroyed, and a validator to event so trying to destroy an eventdate that has been billed for raises an error.
NoRePercussions
force-pushed
the
bugs/timecard-referential-integrity
branch
from
7c8f252
to
b562178
Compare
|
Form-level validation is fixed. My theory is that the destroyed eventdates are being removed from the event before they are validated, thus no form errors. I added another validator to event that checks each eventdate that will be destroyed. |
|
I am hesitant to go forward with this. If these constraints have not been implemented for so long then there is a strong chance that adding them will expose some missing data in Tracker. I was having lot of issues with these commits: 1a918f5 and 60a3d3d. I ultimately had to disable the check in prod because looping over the events in the migration would fail due to validations created after those events: a025884 I think that this and some of your other pulls warrant a return of a full dev instance of Tracker (i.e. copy the prod DB to a dev instance). |
Prevent inconsistency when eventdates are deleted by enforcing that timecard entries must reference a valid eventdate.
This commit alone does not update the UI in response to this change. At minimum, the delete eventdate button should be disabled if there is at least one timecard.