Skip to content

Commit

Permalink
refactor(soft-delete-student-class): fix issue code quality
Browse files Browse the repository at this point in the history
  • Loading branch information
phungmanhcuong committed Oct 15, 2024
1 parent 4c89d59 commit 99f1231
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 18 deletions.
12 changes: 7 additions & 5 deletions app/controllers/course/enrol_requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,20 @@ def create_course_user
create_new_course_user
end

private

def restore_existing_course_user(existing_course_user)
existing_course_user.restore
@enrol_request.update(approve: true)
existing_course_user

Check warning on line 66 in app/controllers/course/enrol_requests_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/enrol_requests_controller.rb#L64-L66

Added lines #L64 - L66 were not covered by tests
end

def create_new_course_user
course_user = CourseUser.new(course_user_params.reverse_merge(
course: current_course, user_id: @enrol_request.user_id, timeline_algorithm: current_course.default_timeline_algorithm
))
course_user = CourseUser.new(
course_user_params.reverse_merge(
course: current_course,
user_id: @enrol_request.user_id,
timeline_algorithm: current_course.default_timeline_algorithm
)
)

CourseUser.transaction do
raise ActiveRecord::Rollback unless course_user.save && @enrol_request.update(approve: true)
Expand Down
28 changes: 15 additions & 13 deletions app/models/course_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ class CourseUser < ApplicationRecord
# A set of roles which comprise the managers of a course.
MANAGER_ROLES = Set[:manager, :owner].map { |v| roles[v] }.freeze

ASSOCIATED_MODELS = %i[
experience_points_records
learning_rate_records
course_user_achievements
email_unsubscriptions
group_users
personal_times
].freeze

validates :role, presence: true
validates :name, length: { maximum: 255 }, presence: true
validates :phantom, inclusion: { in: [true, false] }
Expand Down Expand Up @@ -179,27 +188,20 @@ def self.user?(user)

# Soft delete the course user.
def destroy
experience_points_records.each(&:destroy) if experience_points_records.exists?
learning_rate_records.each(&:destroy) if learning_rate_records.exists?
course_user_achievements.each(&:destroy) if course_user_achievements.exists?
email_unsubscriptions.each(&:destroy) if email_unsubscriptions.exists?
group_users.each(&:destroy) if group_users.exists?
personal_times.each(&:destroy) if personal_times.exists?
ASSOCIATED_MODELS.each do |association|
send(association).destroy_all
end
super
end

# Restore the course user.
def restore
super
experience_points_records.with_deleted.each(&:restore) if experience_points_records.with_deleted.exists?
learning_rate_records.with_deleted.each(&:restore) if learning_rate_records.with_deleted.exists?
course_user_achievements.with_deleted.each(&:restore) if course_user_achievements.with_deleted.exists?
email_unsubscriptions.with_deleted.each(&:restore) if email_unsubscriptions.with_deleted.exists?
group_users.with_deleted.each(&:restore) if group_users.with_deleted.exists?
personal_times.with_deleted.each(&:restore) if personal_times.with_deleted.exists?
ASSOCIATED_MODELS.each do |association|
send(association).with_deleted.restore_all

Check warning on line 201 in app/models/course_user.rb

View check run for this annotation

Codecov / codecov/patch

app/models/course_user.rb#L199-L201

Added lines #L199 - L201 were not covered by tests
end
end


# Test whether this course_user is a manager (i.e. manager or owner)
#
# @return [Boolean] True if course_user is a staff
Expand Down

0 comments on commit 99f1231

Please sign in to comment.