From b712851b351e8f9d1388048507739951a3f86c7b Mon Sep 17 00:00:00 2001 From: fosterfarrell9 <28628554+fosterfarrell9@users.noreply.github.com> Date: Fri, 16 Aug 2024 18:38:08 +0200 Subject: [PATCH] Add redemption and claim models and rewrite workflow accordingly --- app/abilities/voucher_ability.rb | 2 +- app/controllers/concerns/notifier.rb | 10 ++ app/controllers/lectures_controller.rb | 91 +------------------ app/controllers/vouchers_controller.rb | 57 +++++++++++- app/helpers/notifications_helper.rb | 68 +++++++++++--- app/models/claim.rb | 8 ++ app/models/contract.rb | 11 --- app/models/lecture.rb | 28 +++--- app/models/notification.rb | 71 +++++++-------- app/models/redemption.rb | 21 +++++ app/models/tutorial.rb | 2 + app/models/user.rb | 4 +- app/models/voucher.rb | 4 + ...cher.html.erb => _verify_voucher.html.erb} | 4 +- app/views/profile/edit.html.erb | 2 +- .../vouchers/_redeem_editor_voucher.html.erb | 5 +- .../vouchers/_redeem_tutor_voucher.html.erb | 10 +- .../vouchers/{redeem.js.erb => verify.js.erb} | 0 config/locales/de.yml | 4 + config/locales/en.yml | 4 + config/routes.rb | 4 + db/migrate/20240815143156_create_contracts.rb | 11 --- ...40815174623_add_details_to_notification.rb | 5 - .../20240816142403_create_redemptions.rb | 9 ++ db/migrate/20240816150011_create_claims.rb | 10 ++ db/schema.rb | 37 +++++--- 26 files changed, 273 insertions(+), 209 deletions(-) create mode 100644 app/controllers/concerns/notifier.rb create mode 100644 app/models/claim.rb delete mode 100644 app/models/contract.rb create mode 100644 app/models/redemption.rb rename app/views/profile/{_redeem_voucher.html.erb => _verify_voucher.html.erb} (71%) rename app/views/vouchers/{redeem.js.erb => verify.js.erb} (100%) delete mode 100644 db/migrate/20240815143156_create_contracts.rb delete mode 100644 db/migrate/20240815174623_add_details_to_notification.rb create mode 100644 db/migrate/20240816142403_create_redemptions.rb create mode 100644 db/migrate/20240816150011_create_claims.rb diff --git a/app/abilities/voucher_ability.rb b/app/abilities/voucher_ability.rb index 81b94738f..46522ba8f 100644 --- a/app/abilities/voucher_ability.rb +++ b/app/abilities/voucher_ability.rb @@ -8,6 +8,6 @@ def initialize(user) user.can_update_personell?(voucher.lecture) end - can :redeem, Voucher + can [:verify, :redeem], Voucher end end diff --git a/app/controllers/concerns/notifier.rb b/app/controllers/concerns/notifier.rb new file mode 100644 index 000000000..20f24fc16 --- /dev/null +++ b/app/controllers/concerns/notifier.rb @@ -0,0 +1,10 @@ +module Notifier + extend ActiveSupport::Concern + + def notify_new_editor_by_mail(editor, lecture) + NotificationMailer.with(recipient: editor, + locale: editor.locale, + lecture: lecture) + .new_editor_email.deliver_later + end +end diff --git a/app/controllers/lectures_controller.rb b/app/controllers/lectures_controller.rb index 039dcb257..11a497f47 100644 --- a/app/controllers/lectures_controller.rb +++ b/app/controllers/lectures_controller.rb @@ -1,5 +1,6 @@ # LecturesController class LecturesController < ApplicationController + include Notifier include ActionController::RequestForgeryProtection before_action :set_lecture, except: [:new, :create, :search] before_action :set_lecture_cookie, only: [:show, :organizational, @@ -105,7 +106,7 @@ def update recipients = User.where(id: new_ids) recipients.each do |r| - notify_new_editor_by_mail(r) + notify_new_editor_by_mail(r, @lecture) end end @@ -296,27 +297,6 @@ def import_toc redirect_to edit_lecture_path(@lecture) end - def become_tutor - if Voucher.check_voucher(become_tutor_params[:voucher_hash]) - selected_tutorials = @lecture.tutorials.where(id: become_tutor_params[:tutorial_ids]) - @lecture.update_tutor_status!(current_user, selected_tutorials) - create_became_tutor_notification(selected_tutorials) - redirect_to edit_profile_path, notice: I18n.t("controllers.become_tutor_success") - else - handle_invalid_voucher - end - end - - def become_editor - if Voucher.check_voucher(become_editor_params[:voucher_hash]) - @lecture.update_editor_status!(current_user) - notify_new_editor_by_mail(current_user) - redirect_to edit_profile_path, notice: I18n.t("controllers.become_editor_success") - else - handle_invalid_voucher - end - end - private def set_lecture @@ -475,71 +455,4 @@ def check_if_enough_questions redirect_to :root, alert: I18n.t("controllers.no_test") end - - def become_tutor_params - params.permit(:voucher_hash, tutorial_ids: []) - end - - def become_editor_params - params.permit(:voucher_hash) - end - - def handle_invalid_voucher - error_message = I18n.t("controllers.voucher_invalid") - respond_to do |format| - format.js { render "error", locals: { error_message: error_message } } - format.html { redirect_to edit_profile_path, alert: error_message } - end - end - - def notify_new_editor_by_mail(editor) - NotificationMailer.with(recipient: editor, - locale: editor.locale, - lecture: @lecture) - .new_editor_email.deliver_later - end - - # def create_became_tutor_notification(selected_tutorials) - # @lecture.editors_and_teacher.each do |editor| - # details = I18n.t("notifications.became_tutor", user: current_user.info, - # locale: editor.locale) - # if selected_tutorials - # details << I18n.t("notifications.tutorial_details", - # tutorials: selected_tutorials.map(&:title).join(", "), - # locale: editor.locale) - # end - # Notification.create(recipient: editor, - # notifiable: @lecture, - # action: "redemption", - # details: details) - # end - # end - - def create_became_tutor_notification(selected_tutorials) - @lecture.editors_and_teacher.each do |editor| - details = build_notification_details(editor, selected_tutorials) - create_notification(editor, details) - end - end - - def build_notification_details(editor, selected_tutorials) - details = I18n.t("notifications.became_tutor", user: current_user.info, - locale: editor.locale) - if selected_tutorials - tutorial_titles = selected_tutorials.map(&:title).join(", ") - details << I18n.t("notifications.tutorial_details", - tutorials: tutorial_titles, - locale: editor.locale) - end - details - end - - def create_notification(editor, details) - Notification.create( - recipient: editor, - notifiable: @lecture, - action: "redemption", - details: details - ) - end end diff --git a/app/controllers/vouchers_controller.rb b/app/controllers/vouchers_controller.rb index 455fdb05f..a5c24a818 100644 --- a/app/controllers/vouchers_controller.rb +++ b/app/controllers/vouchers_controller.rb @@ -1,5 +1,6 @@ # app/controllers/vouchers_controller.rb class VouchersController < ApplicationController + include Notifier before_action :set_voucher, only: [:invalidate] authorize_resource except: :create @@ -29,7 +30,7 @@ def invalidate end end - def redeem + def verify @voucher = Voucher.check_voucher(params[:voucher_hash]) respond_to do |format| if @voucher @@ -43,12 +44,28 @@ def redeem end end + def redeem + voucher = Voucher.check_voucher(redeem_voucher_params[:voucher_hash]) + if voucher + lecture = voucher.lecture + redemption = process_voucher(voucher, lecture) + redemption.create_notifications! + redirect_to edit_profile_path, notice: success_message(voucher) + else + handle_invalid_voucher + end + end + private def voucher_params params.permit(:lecture_id, :sort) end + def redeem_voucher_params + params.permit(:voucher_hash, tutorial_ids: []) + end + def set_voucher @voucher = Voucher.find_by(id: params[:id]) return if @voucher @@ -62,6 +79,36 @@ def set_related_data I18n.locale = @lecture.locale end + def process_voucher(voucher, lecture) + if voucher.tutor? + process_tutor_voucher(voucher, lecture) + elsif voucher.editor? + process_editor_voucher(voucher, lecture) + end + end + + def process_tutor_voucher(voucher, lecture) + selected_tutorials = lecture.tutorials + .where(id: redeem_voucher_params[:tutorial_ids]) + lecture.update_tutor_status!(current_user, selected_tutorials) + Redemption.create(user: current_user, voucher: voucher, + claimed_tutorials: selected_tutorials) + end + + def process_editor_voucher(voucher, lecture) + lecture.update_editor_status!(current_user) + notify_new_editor_by_mail(current_user, lecture) + Redemption.create(user: current_user, voucher: voucher) + end + + def success_message(voucher) + if voucher.tutor? + I18n.t("controllers.become_tutor_success") + elsif voucher.editor? + I18n.t("controllers.become_editor_success") + end + end + def handle_successful_save(format) format.html { redirect_to edit_lecture_path(@lecture, anchor: "people") } format.js @@ -92,4 +139,12 @@ def handle_voucher_not_found end end end + + def handle_invalid_voucher + error_message = I18n.t("controllers.voucher_invalid") + respond_to do |format| + format.js { render "error", locals: { error_message: error_message } } + format.html { redirect_to edit_profile_path, alert: error_message } + end + end end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index fb2bedab2..d574109f1 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -18,7 +18,7 @@ def notification_menu_item_details(notification) return medium_notification_item_details(notifiable) if notification.medium? return course_notification_item_details(notifiable) if notification.course? return lecture_notification_item_details(notifiable) if notification.lecture? - return redemption_notification_item_details(notification) if notification.redemption? + return redemption_notification_item_details(notifiable) if notification.redemption? "" end @@ -77,7 +77,7 @@ def notification_link(notification) elsif notification.lecture? lecture_notification_card_link elsif notification.redemption? - notification.details + redemption_notification_details(notifiable) else notifiable.details end @@ -91,24 +91,64 @@ def items_card_size(small, comments_below) end # create text for lecture announcement in notification card header - def redemption_notification_card_header(lecture) - link_to(lecture.title_for_viewers, - edit_lecture_path(lecture, anchor: "people"), + def redemption_notification_card_header(redemption) + link_to(redemption.lecture.title_for_viewers, + edit_lecture_path(redemption.lecture, anchor: "people"), class: "text-dark") end - def redemption_notification_item_header(lecture) - t("notifications.redemption_in_lecture", lecture: lecture.title_for_viewers) + def redemption_notification_item_header(redemption) + t("notifications.redemption_in_lecture", + lecture: redemption.lecture.title_for_viewers) end - def redemption_notification_item_details(notification) - extract_name_from_redemption_details(notification.details) + def redemption_notification_details(redemption) + if redemption.tutor? + details = I18n.t("notifications.became_tutor", user: redemption.user.info) + tutorial_titles = redemption.claimed_tutorials.map(&:title).join(", ") + details << I18n.t("notifications.tutorial_details", + tutorials: tutorial_titles) + else + I18n.t("notifications.became_editor", user: redemption.user.info) + end end - # this a admittedly a hack but I did not want to add another column to - # the notifications table - def extract_name_from_redemption_details(details) - match_data = details.match(/^(.+?) \(.+\)/) - match_data ? match_data[1] : nil + def redemption_notification_item_details(redemption) + result = if redemption.tutor? + tutor_notification_item_details(redemption) + elsif redemption.editor? + editor_notification_item_details(redemption) + end + + truncate_result(result) end + + private + + def tutor_notification_item_details(redemption) + tutorials = redemption.claimed_tutorials.map(&:title).join(", ") + "#{t("basics.tutor")} #{redemption.user.tutorial_name}: #{tutorials}" + end + + def editor_notification_item_details(redemption) + "#{t("basics.editor")} #{redemption.user.tutorial_name}" + end + + def tutor_notification_details(redemption) + details = I18n.t("notifications.became_tutor", + user: redemption.user.info) + tutorial_titles = redemption.claimed_tutorials.map(&:title).join(", ") + details << I18n.t("notifications.tutorial_details", + tutorials: tutorial_titles) + end + + def editor_notification_details(redemption) + I18n.t("notifications.became_editor", user: redemption.user.info) + end + + def truncate_result(result) + result.first(40).tap do |truncated| + return truncated.length < 40 ? truncated : truncated + "..." + end + end end diff --git a/app/models/claim.rb b/app/models/claim.rb new file mode 100644 index 000000000..dd209e3df --- /dev/null +++ b/app/models/claim.rb @@ -0,0 +1,8 @@ +# Claim class +# claims store what is beign taken over by the user when they redeem a voucher +# (e.g. a tutorial or a talk) + +class Claim < ApplicationRecord + belongs_to :redemption + belongs_to :claimable, polymorphic: true +end diff --git a/app/models/contract.rb b/app/models/contract.rb deleted file mode 100644 index cf1cd1f98..000000000 --- a/app/models/contract.rb +++ /dev/null @@ -1,11 +0,0 @@ -class Contract < ApplicationRecord - belongs_to :user - belongs_to :lecture - - scope :tutors, -> { where(role: ROLE_HASH[:tutor]) } - scope :editors, -> { where(role: ROLE_HASH[:editor]) } - - ROLE_HASH = { tutor: 0, editor: 1 }.freeze - - enum role: ROLE_HASH -end diff --git a/app/models/lecture.rb b/app/models/lecture.rb index 4cc9cf0b1..bec8bdbfe 100644 --- a/app/models/lecture.rb +++ b/app/models/lecture.rb @@ -864,7 +864,7 @@ def update_tutor_status!(user, selected_tutorials) remove_tutor(t, user) end end - Contract.create(user: user, lecture: self, role: :tutor) + # Contract.create(user: user, lecture: self, role: :tutor) # touch to invalidate the cache touch end @@ -873,33 +873,37 @@ def update_editor_status!(user) return if editors.include?(user) editors << user - Contract.create(user: user, lecture: self, role: :editor) + # Contract.create(user: user, lecture: self, role: :editor) # touch to invalidate the cache touch end + def voucher_redeemers(voucher_scope) + User.where(id: Redemption.where(voucher: voucher_scope).select(:user_id)) + end + + def tutors_by_redemption + voucher_redeemers(vouchers.for_tutors) + end + + def editors_by_redemption + voucher_redeemers(vouchers.for_editors) + end + def eligible_as_tutors - (tutors + tutors_with_contract + editors + [teacher]).uniq + (tutors + tutors_by_redemption + editors + [teacher]).uniq # the first one should (in the future) actually be contained in the sum of # the other ones, but in the transition phase where some tutor statuses were # still given by the old system, this will not be true end def eligible_as_editors - (editors + editors_with_contract + course.editors - [teacher]).uniq + (editors + editors_by_redemption + course.editors - [teacher]).uniq # the first one should (in the future) actually be contained in the sum of # the other ones, but in the transition phase where some editor statuses were # still given by the old system, this will not be true end - def tutors_with_contract - User.where(id: Contract.where(lecture: self, role: :tutor).select(:user_id)) - end - - def editors_with_contract - User.where(id: Contract.where(lecture: self, role: :editor).select(:user_id)) - end - def editors_and_teacher [teacher] + editors end diff --git a/app/models/notification.rb b/app/models/notification.rb index fda36e0dc..669d23865 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -9,20 +9,12 @@ class Notification < ApplicationRecord paginates_per 12 - # retrieve notifiable defined by notifiable_type and notifiable_id - # def notifiable - # return unless notifiable_type.in?(Notification.allowed_notifiable_types) && - # notifiable_id.present? - # notifiable_type.constantize.find_by_id(notifiable_id) - # end - # returns the lecture associated to a notification of type announcement, # and teachable for a notification of type medium, nil otherwise def teachable return if notifiable.blank? - return notifiable.lecture if notifiable_type == "redemption" - return if notifiable_type.in?(["Lecture", "Course"]) - return notifiable.lecture if notifiable_type == "Announcement" + return if lecture_or_course? + return notifiable.lecture if announcement_or_redemption? # notifiable will be a medium, so return its teachable notifiable.teachable @@ -35,53 +27,42 @@ def teachable # all other cases: notifiable path def path(user) return if notifiable.blank? - return edit_lecture_path(notifiable, anchor: "people") if action == "redemption" - return edit_profile_path if notifiable_type.in?(["Course", "Lecture"]) - - if notifiable_type == "Announcement" - return notifiable.lecture.path(user) if notifiable.lecture.present? - return news_path + if redemption? + edit_lecture_path(notifiable.lecture, anchor: "people") + elsif lecture_or_course? + edit_profile_path + elsif lecture_announcement? + notifiable.lecture.path(user) + elsif generic_announcement? + news_path + elsif quiz? + medium_path(notifiable) + else + polymorphic_url(notifiable, only_path: true) end - return medium_path(notifiable) if notifiable_type == "Medium" && notifiable.sort == "Quiz" - - polymorphic_url(notifiable, only_path: true) - end - - def self.allowed_notifiable_types - ["Medium", "Course", "Lecture", "Announcement"] end # the next methods are for the determination which kind of notification it is def medium? - return false if notifiable.blank? - - notifiable_type == "Medium" + notifiable.is_a?(Medium) end def course? - return false if notifiable.blank? - - notifiable.instance_of?(::Course) + notifiable.is_a?(Course) end def lecture? - return false if notifiable.blank? - - notifiable.instance_of?(::Lecture) && action != "redemption" + notifiable.is_a?(Lecture) end def redemption? - return false if notifiable.blank? - - action == "redemption" + notifiable.is_a?(Redemption) end def announcement? - return false if notifiable.blank? - - notifiable.instance_of?(::Announcement) + notifiable.is_a?(Announcement) end def generic_announcement? @@ -91,4 +72,18 @@ def generic_announcement? def lecture_announcement? announcement? && notifiable.lecture.present? end + + def quiz? + medium? && notifiable.sort == "Quiz" + end + + private + + def lecture_or_course? + notifiable_type.in?(["Lecture", "Course"]) + end + + def announcement_or_redemption? + notifiable_type.in?(["Announcement", "Redemption"]) + end end diff --git a/app/models/redemption.rb b/app/models/redemption.rb new file mode 100644 index 000000000..093c3bfe8 --- /dev/null +++ b/app/models/redemption.rb @@ -0,0 +1,21 @@ +# Redempetion class +# redemptions store the event of a user redeeming a voucher + +class Redemption < ApplicationRecord + belongs_to :voucher + belongs_to :user + has_many :claims, dependent: :destroy + has_many :claimed_tutorials, through: :claims, source: :claimable, + source_type: "Tutorial" + + delegate :lecture, to: :voucher + delegate :sort, to: :voucher + delegate :tutor?, to: :voucher + delegate :editor?, to: :voucher + + def create_notifications! + lecture.editors_and_teacher.each do |editor| + Notification.create(notifiable: self, recipient: editor) + end + end +end diff --git a/app/models/tutorial.rb b/app/models/tutorial.rb index 824f390cf..9e3dcd6fb 100644 --- a/app/models/tutorial.rb +++ b/app/models/tutorial.rb @@ -9,6 +9,8 @@ class Tutorial < ApplicationRecord has_many :submissions, dependent: :destroy + has_many :claims, as: :claimable, dependent: :destroy + before_destroy :check_destructibility, prepend: true # rubocop:todo Rails/UniqueValidationWithoutIndex diff --git a/app/models/user.rb b/app/models/user.rb index 57be5375a..35cc87d0b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -81,8 +81,8 @@ class User < ApplicationRecord has_many :feedbacks, dependent: :destroy - # a user has contracts as asssistant in lectures - has_many :contracts, dependent: :destroy + # a user has redemptions of vouchers + has_many :redemptions, dependent: :destroy include ScreenshotUploader[:image] diff --git a/app/models/voucher.rb b/app/models/voucher.rb index f22811aea..606ad7abe 100644 --- a/app/models/voucher.rb +++ b/app/models/voucher.rb @@ -5,6 +5,8 @@ class Voucher < ApplicationRecord belongs_to :lecture, touch: true before_create :generate_secure_hash + has_many :redemptions, dependent: :destroy + before_create :add_expiration_datetime before_create :ensure_no_other_active_voucher validates :sort, presence: true @@ -13,6 +15,8 @@ class Voucher < ApplicationRecord where("expires_at > ? AND invalidated_at IS NULL", Time.zone.now) } + scope :for_tutors, -> { where(sort: :tutor) } + scope :for_editors, -> { where(sort: :editor) } self.implicit_order_column = "created_at" diff --git a/app/views/profile/_redeem_voucher.html.erb b/app/views/profile/_verify_voucher.html.erb similarity index 71% rename from app/views/profile/_redeem_voucher.html.erb rename to app/views/profile/_verify_voucher.html.erb index a9278a5f0..40e97aebe 100644 --- a/app/views/profile/_redeem_voucher.html.erb +++ b/app/views/profile/_verify_voucher.html.erb @@ -1,9 +1,9 @@ -<%= form_with url: redeem_voucher_path, +<%= form_with url: verify_voucher_path, remote: true, method: :post, html: { class: "form-inline" } do |f| %>
<%= t('profile.become_tutor', lecture: voucher.lecture.title) %>
-<%= form_with url: become_tutor_path(voucher.lecture), - remote: true, - method: :post, - html: { class: "form-inline" } do |f| %> +<%= form_with url: redeem_voucher_path, + remote: true, + method: :post, + html: { class: "form-inline" } do |f| %> <%= f.hidden_field :voucher_hash, value: voucher.secure_hash %>