Skip to content

Commit

Permalink
association-improvement
Browse files Browse the repository at this point in the history
Added dependent: :destroy to all the associations that needed it (in the Assignment and Assessment models), changed the order of associations in Assignment to make groups-and-groupings first (this is the first thing  that should be checked when deleting an assignment), created a new locale string for use in the AnnotationCategory model, and added another rescue block in the #destroy action of assignments controller to handle other potential complications that could arise in assignment deletion. This last one is temporary, since we would ideally like to perform a more targetted handling of different exceptions (but for now this prevents crashing and gives an idea of the problem).
  • Loading branch information
mimischly7 committed Mar 1, 2024
1 parent 0189ba6 commit 3fdde26
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 16 deletions.
3 changes: 3 additions & 0 deletions app/controllers/assignments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,9 @@ def destroy
rescue ActiveRecord::DeleteRestrictionError
flash_message(:error, I18n.t('assignments.assignment_has_groupings'))
redirect_back fallback_location: { action: :edit, id: @assignment.id }
rescue StandardError => e
flash_message(:error, "Problem with deletion: #{e.message}")
redirect_back fallback_location: { action: :edit, id: @assignment.id }
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/models/annotation_category.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def delete_allowed?

def check_if_marks_released
if marks_released?
errors.add(:base, 'Cannot update annotation category flexible criterion once results are released')
errors.add(:base, I18n.t('activerecord.errors.models.annotation_category.cannot_update_flex'))
throw(:abort)
end
end
Expand Down
9 changes: 6 additions & 3 deletions app/models/assessment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@ class Assessment < ApplicationRecord
scope :grade_entry_forms, -> { where(type: 'GradeEntryForm') }

has_many :marking_weights, dependent: :destroy
has_many :tags
has_many :tags, dependent: :destroy
belongs_to :course, inverse_of: :assessments

has_many :assessment_section_properties, inverse_of: :assessment, class_name: 'AssessmentSectionProperties'
has_many :assessment_section_properties,
dependent: :destroy,
inverse_of: :assessment,
class_name: 'AssessmentSectionProperties'
accepts_nested_attributes_for :assessment_section_properties

has_many :lti_line_items
has_many :lti_line_items, dependent: :destroy

# Call custom validator in order to validate the :due_date attribute
# date: true maps to DateValidator (custom_name: true maps to CustomNameValidator)
Expand Down
33 changes: 21 additions & 12 deletions app/models/assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ class Assignment < Assessment

validates :due_date, presence: true

# this has to be before :peer_reviews or it throws a HasManyThroughOrderError
has_many :groupings, foreign_key: :assessment_id, inverse_of: :assignment
has_many :groups, through: :groupings, dependent: :restrict_with_exception

has_one :assignment_properties,
dependent: :destroy,
inverse_of: :assignment,
Expand All @@ -32,12 +36,14 @@ class Assignment < Assessment
has_many :ta_criteria,
-> { where(ta_visible: true).order(:position) },
class_name: 'Criterion',
dependent: :destroy,
inverse_of: :assignment,
foreign_key: :assessment_id

has_many :peer_criteria,
-> { where(peer_visible: true).order(:position) },
class_name: 'Criterion',
dependent: :destroy,
inverse_of: :assignment,
foreign_key: :assessment_id

Expand All @@ -57,32 +63,35 @@ class Assignment < Assessment
accepts_nested_attributes_for :assignment_files, allow_destroy: true
validates_associated :assignment_files

# this has to be before :peer_reviews or it throws a HasManyThroughOrderError
has_many :groupings, foreign_key: :assessment_id, inverse_of: :assignment
# Assignments can now refer to themselves, where this is null if there
# is no parent (the same holds for the child peer reviews)
belongs_to :parent_assignment,
class_name: 'Assignment', optional: true, inverse_of: :pr_assignment, foreign_key: :parent_assessment_id
has_one :pr_assignment, class_name: 'Assignment', foreign_key: :parent_assessment_id, inverse_of: :parent_assignment
has_many :peer_reviews, through: :groupings
has_many :pr_peer_reviews, through: :parent_assignment, source: :peer_reviews
has_one :pr_assignment,
class_name: 'Assignment',
dependent: :destroy,
foreign_key: :parent_assessment_id,
inverse_of: :parent_assignment
has_many :peer_reviews, dependent: :destroy, through: :groupings
has_many :pr_peer_reviews, through: :parent_assignment, source: :peer_reviews, dependent: :destroy

has_many :current_submissions_used, through: :groupings,
source: :current_submission_used
has_many :current_submissions_used,
through: :groupings,
source: :current_submission_used,
dependent: :destroy

has_many :ta_memberships, through: :groupings
has_many :student_memberships, through: :groupings
has_many :ta_memberships, through: :groupings, dependent: :destroy
has_many :student_memberships, through: :groupings, dependent: :destroy

has_many :submissions, through: :groupings
has_many :groups, through: :groupings, dependent: :restrict_with_exception
has_many :submissions, through: :groupings, dependent: :destroy

has_many :notes, as: :noteable, dependent: :destroy

has_many :exam_templates, dependent: :destroy, inverse_of: :assignment, foreign_key: :assessment_id

has_many :starter_file_groups, dependent: :destroy, inverse_of: :assignment, foreign_key: :assessment_id

has_many :tas, -> { distinct }, through: :ta_memberships, source: :role
has_many :tas, -> { distinct }, through: :ta_memberships, source: :role, dependent: :destroy

before_save do
@prev_assessment_section_property_ids = assessment_section_properties.ids
Expand Down
4 changes: 4 additions & 0 deletions config/locales/models/annotation_categories/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ en:
annotation_category:
annotation_category_name: Category name
flexible_criterion: Associated Criterion (Flexible type only)
errors:
models:
annotation_category:
cannot_update_flex: Cannot update annotation category flexible criterion once results are released
models:
annotation_category:
one: Annotation Category
Expand Down

0 comments on commit 3fdde26

Please sign in to comment.