Skip to content

Commit

Permalink
Vouchers for user promotion - Part 2: Redemption (#671)
Browse files Browse the repository at this point in the history
* Initialize voucher model and add some unit tests

* Make ensure_no_other_active_voucher validation into a callback

* Add throw :abort in order to halt execution

* Replace Time.now by Time.zone.now

* Set up basic functionality for display of vouchers for tutors

* Create separate file for copy and paste button code and decaffeinate

* Set up destruction of tutor vouchers

* Rename view file as it containes embedded ruby

* Add create action for vouchers and corresponding views

* Adapt views and controller for adding and removing of vouchers of different type

* Put duplicate lines into a separate method

* Set up redeeming of vouchers

* fix typo

* remove obsolete methods

* Avoid use of Time.now

* Refactor active_vouhcer_of_sort method

* remove unused expired? method

* Remove duplicate code

* remove unused variable

* Add controller spec for vouchers

* Rewrite controller spec for vouchers

* remove obsolete comment

* Set up redemption of tutor vouchers

* Remove user lookup for tutor positions

* Set up redemption of editor vouchers

* Add Contract Model and setup selection of tutors and editors

* Set up notifications for redemption of tutor vouchers

* Invalidate vouchers instead of destroying them

* Add redemption and claim models and rewrite workflow accordingly

* Restore to previous state

* Fix typo

* Implement Rubocop suggestion

* Replace voucher_hash by secure_hash

* Replace redeem_voucher_params by check_voucher_params

* Add cancel action to vouchers controller

* Differentiate between different cases of tutor voucher redemption

* Differentiate between different cases of editor voucher redemption

* Take first steps in setting up redemption of teacher vouchers

* Add notifications for redemption of teacher vouchers

* Add notification mails for teacher change by voucher redemption

* Invalidate teacher vouchers after first use

* Add vouchers for seminar speakers

* Add helpdesk for speaker vouchers

* Set up redemption of speaker vouchers

* Add notifications for redemption of speaker vouchers

* Restrict access user select

* Remove unused parameter

* Introduce VoucherProcessor service object

* Use common partial for redemption of tutor and speeaker vouchers

* Rename sort attribute of vouchers to role

* Rename remaining occurences of sort attribute of voucher to role

* Add unit tests for VoucherProcessor

* Add .uniq to method

* Remove teacher selection for non-admins

* Remove obsolete reference to non-existent model

* Remove permissions from non-admins to change teachers

* Add cypress data attributes

* Add first cypress tests

* Add more cypress tests

* Add first cypress tests for voucher redemption

* Init future possibility to check clipboard content

* Add missing teacher field for non-admins

* Add cypress tests for redemption form

* Add cypress command .tomselect to select in TomSelect forms

* Add test for redemption form using the new .tomselect command

* Remove unnecessary call of trait

* Use NO_SEMINAR_ROLES constant

* Clean up some experiments

* Remove .tomselect Command for cypress

* Enlarge test user json by some data

* Add logout command and expect correct statuses for login and logout requests

* Add more cypress tests for redemption of tutor vouchers

* Parse hashes with integer keys into arrays of strings

* Add tests for tutor voucher redemption in the case that all tutorials are already taken

* Add possibility to pass instance methods and refactor

* Update documentation

* Add tests for voucher redemption that check notifications

* Add missing speaker voucher case

* Add missing locales

* Add cypress tests fo editor vouchers, teacher vouchers and speaker vouchers

* Remove out of scope test

* Refactor helper methods

* Extract helper functions to separate file

* Refactor tests

* Extract more methods into helper

* Rename files according to conventions

* Add controller specs for changed lecture update policy

* Refactor tests by extracting methods

* Don't define `redeem` method twice

Apparently I missed that one during the merge.

* Pluralize voucher redemption spec filename

* Delete duplicate migration & update new migration dates/schema

* Move cypress helpers to e2e folder

`support` folder should be used for general test-stuff, i.e. not-related
to individual, specific tests.

* Improve naming for voucher finding method

also moved one private function around

* Inline JSON object in user creator controller

* Use symbolic keys upon voucher type case statement

* Fix "Redeem voucher" spelling mistake

* Use fixed width for voucher redemption selectize field

* Rename view file to `voucher_redemption_form`

* Fix indentation in view file (tutor voucher)

* Replace cypress `describe` by `context`

* Remove reviewer-TODO comments (selectize tomselect)

* Reset Cypress-related code to 49cf16a

Command used:
git checkout 49cf16a -- ./.vscode/settings.json ./app/controllers/cypress/factories_controller.rb ./app/controllers/cypress/user_creator_controller.rb ./spec/cypress/support/factorybot.js

* Use new .call syntax in cypress tests

(in order to call instance methods, see #696

* Explicitly set `name_in_tutorials` for users

* Remove constraint that user id must be a number

This is such that UUIDs also work.

* Init support for entity-relationship-diagram creation

* Add `has_many :notifications` for better ERD visibility

* Remove usage of delegate methods from redemption

* Don't hardcode source_type as string

* Refactor voucher_processor into model concern

According to concepts discussed in #694.
Also auto-load subdirectories in models folder
and set Current.user for usage in models.

* Add missing notification and subscribe lecture steps

* Move vouchers down in UI (lecture edit page)

* Turn off autocomplete for redeem voucher text field

* Strip secure hash to avoid copy-pasting issues with voucher

* Improve voucher-model docstrings

* Fix wrong permit syntax for arrays

* Move Claim model to voucher folder

* Adapt Redeemer specs to new architecture

Also rename Redeemable to Redeemer to better reflect what it's doing.

* Use `pluck` instead of `select` for better performance

Also use .uniq just to be sure.

* Test timestamp in invalidate! voucher spec

* Remove unused routes

* Fix missing CSpell configuration

* Improve tutor voucher redemption messages

also remove unused locale keys

* Improve speaker voucher redemption messages

* Replace "Voucher" by "Gutschein" in German texts

* Improve teacher voucher redemption messages

* Improve editor voucher redemption messages

* Remove duplicate `no_active_voucher` i18n key

* Add help texts to voucher creation

* Also eager_load additional modules

This is to ensure that `Rails.application.eager_load!` in the rails_erd
gem respects all our modules.

See this line:
https://github.com/voormedia/rails-erd/blob/7c66258b6818c47b4d878c2ad7ff6decebdf834a/lib/rails_erd/tasks.rake#L45

* Outsource x_by_redemption methods from lecture to redemption

* Fix wrong i18n controller error

The code always errored since the JSON objects are passed as strings
from the frontend.

* Fix cypress test (due to renaming of i18n keys / html)

* Cypress test that whitespaces in voucher hash work

* Verify voucher is invalid after user became teacher

* Add user deletion cypress spec (for tutor)

* Add word to cspell list

* Replace "display" by "show"

For me, "display" is closer to the frontend, while this is a backend test,
that's why I renamed it. Not really that important in the end ;)

* Fix two spelling mistakes (out of scope)

* Move lecture notifications to separate file

* Replace Notifier concern by LectureNotifier module

* Remove unused set_recipients method for "new editor mail"

* Move email templates to right folder

* Correct sender_nad_locale setting

* Introduce enqueue_mail_with_params matcher & test for editor

* Add spec for Current user model

* Inline sender_and_locale

* Try to test email sending for editor voucher (fails)

* Mock Current model in RSpec tests

* Rework sender and locale setting

* Fix mail sending test for editor

* Test mail body (editor)

* Set locale in broader scope test

* Fix comment

* Test previous and new teacher mail

* Use custom html body matcher (ignore \r\n)

* Improve wording in html body failure message

* Test mails for co-speakers

* Ensure user that is now speaker doesn't receive a mail

* Use just one file for mail matchers

* Rename matcher to enqueue_mail_including_params

* Outsource from notification mailer assertion

* Refactor send mail to co-speaker test

* Remove unwanted test

I accidentally added this even though it's not the
wanted behavior.

* Add tests for teacher selection dropdown

* Write test for editor dropdown selection

* Allow admin to select any user in lecture editor select

* Remove `only` from cypress test

* Add more words to spell checker

* Add tests for tutor selection dropdown

* Add back ability for admin to select any user as tutor

* Allow admin to choose arbitrary users as speakers & test

* Remove unnecessary display: none

* Improve if statement in voucher redemption

* Remove another unnecessary "be.visible" statement

* Allow admins to select arbitrary users in existing talks & test

* Rename lecture spec file

* Refactor lecture people select spec (extract methods)

* Reuse speaker_select helper

* Outsource to new teacher_select helper

* Outsource to new editors_select helper

* Remove with_seminar trait

(since we use the association field now, introduced
a few commits beforehand)

* Make cypress input selecting more reliable

* Remove unnecessary div wrap

* Avoid flaky test by intercepting user fill request

* Fix cypress not typing "cy" in input box

* Remove current_user= test as method was removed

* Intercept /talks/new route to avoid flaky test

This is due to the form not being completely loaded while we
already go on.

* Add cy.wait as last resort

* Visit #people page directly

* Remove unwanted `only` in Cypress test

---------

Co-authored-by: Splines <dominic-plein@gmx.de>
  • Loading branch information
fosterfarrell9 and Splines authored Nov 10, 2024
1 parent af47a3f commit 7ba50e8
Show file tree
Hide file tree
Showing 92 changed files with 2,566 additions and 221 deletions.
24 changes: 22 additions & 2 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,32 @@
//////////////////////////////////////
// Spell Checker
//////////////////////////////////////
"cSpell.enabled": true,
"cSpell.ignorePaths": [
"node_modules",
".git"
],
"cSpell.language": "en,de",
"cSpell.words": [
"activerecord",
"ajax",
"commontator",
"cospeaker",
"cospeakers",
"datetime",
"factorybot",
"helpdesk",
"katex",
"preselection",
"selectize",
"Timecop",
"turbolinks"
]
"turbolinks",
"uncached",
"whitespaces"
],
"cSpell.enableFiletypes": [
"ruby"
// Other filetypes are handled by the default spell checker
],
"cSpell.maxNumberOfProblems": 10000
}
6 changes: 1 addition & 5 deletions app/abilities/user_ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,7 @@ def initialize(user)
user.admin? || (!user.generic? && user == given_user)
end

can :fill_user_select, User do
user.active_teachable_editor?
end

can :list_generic_users, User do
can [:fill_user_select, :list_generic_users], User do
user.admin?
end
end
Expand Down
2 changes: 2 additions & 0 deletions app/abilities/voucher_ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,7 @@ def initialize(user)
can [:create, :invalidate], Voucher do |voucher|
user.can_update_personell?(voucher.lecture)
end

can [:verify, :redeem, :cancel], Voucher
end
end
6 changes: 6 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class ApplicationController < ActionController::Base
before_action :authenticate_user!
before_action :set_locale
after_action :store_interaction, if: :user_signed_in?
before_action :set_current_user

etag { current_user.try(:id) }

Expand Down Expand Up @@ -135,4 +136,9 @@ def cookie_locale_param
def available_locales
I18n.available_locales.map(&:to_s)
end

# https://stackoverflow.com/a/69313330/
def set_current_user
Current.user = current_user
end
end
Empty file removed app/controllers/concerns/.keep
Empty file.
10 changes: 5 additions & 5 deletions app/controllers/cypress/i18n_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ def create

substitutions = {}
if params[:substitutions].present?
unless params[:substitutions].is_a?(Hash)
msg = "Argument `substitution` must be a hash indicating the substitutions."
msg += " But we got: '#{params[:substitutions]}'"
begin
substitutions = params[:substitutions].to_unsafe_hash.symbolize_keys
rescue NoMethodError
msg = "Argument `substitution` is '#{params[:substitutions]}'."
msg += " We cannot convert that to a hash."
raise(ArgumentError, msg)
end
substitutions = params[:substitutions].to_unsafe_hash.symbolize_keys
end

i18n_key = params[:i18n_key]

render json: I18n.t(i18n_key, **substitutions), status: :created
end
end
Expand Down
3 changes: 3 additions & 0 deletions app/controllers/cypress/user_creator_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ def create

user = User.create(name: "#{role} Cypress #{random_hash}",
email: "#{role}-#{random_hash}@mampf.cypress",
# Note that some Cypress tests rely on the username
# beginning with "cy" (!)
name_in_tutorials: "cy-#{role}-#{random_hash}",
password: CYPRESS_PASSWORD, consents: true, admin: is_admin,
locale: I18n.default_locale)
user.confirm
Expand Down
45 changes: 45 additions & 0 deletions app/controllers/lecture_notifier.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
module LectureNotifier
extend self

def notify_new_editor_by_mail(editor, lecture)
LectureNotificationMailer.with(recipient: editor,
locale: editor.locale,
lecture: lecture)
.new_editor_email.deliver_later
end

def notify_about_teacher_change_by_mail(lecture, previous_teacher)
notify_new_teacher_by_mail(lecture)
notify_previous_teacher_by_mail(previous_teacher, lecture)
end

def notify_cospeakers_by_mail(speaker, talks)
talks.each do |talk|
talk.speakers.each do |cospeaker|
next if cospeaker == speaker

LectureNotificationMailer.with(recipient: cospeaker,
locale: cospeaker.locale,
talk: talk,
speaker: speaker)
.new_speaker_email.deliver_later
end
end
end

private

def notify_new_teacher_by_mail(lecture)
LectureNotificationMailer.with(recipient: lecture.teacher,
locale: lecture.teacher.locale,
lecture: lecture)
.new_teacher_email.deliver_later
end

def notify_previous_teacher_by_mail(previous_teacher, lecture)
LectureNotificationMailer.with(recipient: previous_teacher,
locale: previous_teacher.locale,
lecture: lecture)
.previous_teacher_email.deliver_later
end
end
15 changes: 7 additions & 8 deletions app/controllers/lectures_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,16 @@ def edit

def create
@lecture = Lecture.new(lecture_params)
@lecture.teacher = current_user unless current_user.admin?
authorize! :create, @lecture
@lecture.save
if @lecture.valid?
@lecture.update(sort: "special") if @lecture.course.term_independent
# set organizational_concept to default
set_organizational_defaults
# set lenguage to default language
# set language to default language
set_language
# depending on where the create action was trriggered from, return
# depending on where the create action was triggered from, return
# to admin index view or edit course view
unless params[:lecture][:from] == "course"
redirect_to administration_path,
Expand Down Expand Up @@ -105,10 +106,7 @@ def update
recipients = User.where(id: new_ids)

recipients.each do |r|
NotificationMailer.with(recipient: r,
locale: r.locale,
lecture: @lecture)
.new_editor_email.deliver_later
LectureNotifier.notify_new_editor_by_mail(r, @lecture)
end
end

Expand Down Expand Up @@ -336,9 +334,10 @@ def lecture_params
:submission_max_team_size, :submission_grace_period,
:annotations_status]
if action_name == "update" && current_user.can_update_personell?(@lecture)
allowed_params.push(:teacher_id, { editor_ids: [] })
allowed_params.push({ editor_ids: [] })
end
allowed_params.push(:course_id, :teacher_id, { editor_ids: [] }) if action_name == "create"
allowed_params.push(:course_id, { editor_ids: [] }) if action_name == "create"
allowed_params.push(:teacher_id) if current_user.admin?
params.require(:lecture).permit(allowed_params)
end

Expand Down
50 changes: 48 additions & 2 deletions app/controllers/vouchers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,35 @@ def invalidate
end
end

def verify
@voucher = Voucher.find_voucher_by_hash(params[:secure_hash])
respond_to do |format|
if @voucher
format.js
format.html { head :no_content }
else
error_message = I18n.t("controllers.voucher_invalid")
format.js { render "error", locals: { error_message: error_message } }
format.html { redirect_to edit_profile_path, alert: error_message }
end
end
end

def redeem
# TODO: this will be dealt with in the corresponding 2nd PR
render js: "alert('Voucher redeemed!')"
voucher = Voucher.find_voucher_by_hash(params[:secure_hash])
if voucher
voucher.redeem(params.permit(tutorial_ids: [], talk_ids: []))
redirect_to edit_profile_path, notice: success_message(voucher)
else
handle_invalid_voucher
end
end

def cancel
respond_to do |format|
format.html { redirect_to edit_profile_path }
format.js
end
end

private
Expand All @@ -50,6 +76,18 @@ def set_related_data
I18n.locale = @lecture.locale
end

def success_message(voucher)
if voucher.tutor?
I18n.t("controllers.become_tutor_success")
elsif voucher.editor?
I18n.t("controllers.become_editor_success")
elsif voucher.teacher?
I18n.t("controllers.become_teacher_success")
elsif voucher.speaker?
I18n.t("controllers.become_speaker_success")
end
end

def handle_successful_save(format)
format.html { redirect_to edit_lecture_path(@lecture, anchor: "people") }
format.js
Expand Down Expand Up @@ -80,4 +118,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
6 changes: 6 additions & 0 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -327,4 +327,10 @@ def get_class_for_any_path(paths)
def get_class_for_any_path_startswith(paths)
paths.any? { |path| request.path.starts_with?(path) } ? ACTIVE_CSS_CLASS : ""
end

def truncate_result(result, length = 40)
result.first(length).tap do |truncated|
return truncated.length < length ? truncated : "#{truncated}..."
end
end
end
77 changes: 77 additions & 0 deletions app/helpers/lectures_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,4 +129,81 @@ def lecture_view_icon(lecture)
tag.i(class: "fas fa-eye")
end
end

def editors_preselection(lecture)
options_for_select(lecture.eligible_as_editors.map do |editor|
[editor.info, editor.id]
end, lecture.editor_ids)
end

def teacher_select(form, is_new_lecture, lecture = nil)
if current_user.admin?
label = form.label(:teacher_id, t("basics.teacher"), class: "form-label")
help_desk = helpdesk(t("admin.lecture.info.teacher"), false)

preselection = if is_new_lecture
options_for_select([[current_user.info, current_user.id]], current_user.id)
else
options_for_select([[lecture.teacher.info, lecture.teacher.id]], lecture.teacher.id)
end

# TODO: Rubocop bug when trying to break the last object on a new line
select = form.select(:teacher_id, preselection, {}, { class: "selectize",
multiple: true,
data: {
ajax: true,
filled: false,
model: "user",
placeholder: t("basics.enter_two_letters"), # rubocop:disable Layout/LineLength
no_results: t("basics.no_results"),
modal: true,
cy: "teacher-admin-select"
} })

error_div = content_tag(:div, "", class: "invalid-feedback", id: "lecture-teacher-error")

return label + help_desk + select + error_div
end

# Non-admin cases
if is_new_lecture
p1 = content_tag(:p) do
concat(t("basics.teacher"))
concat(helpdesk(t("admin.lecture.info.teacher_fixed_new_lecture"), false))
end
p2 = content_tag(:p, current_user.info)

else
p1 = content_tag(:p) do
concat(t("basics.teacher"))
concat(helpdesk(t("admin.lecture.info.teacher_fixed"), false))
end
p2 = content_tag(:p, lecture.teacher.info, "data-cy": "teacher-info")
end

p1 + p2
end

def editors_select(form, lecture)
if current_user.admin?
preselection = options_for_select(lecture.select_editors, lecture.editors.map(&:id))
form.select(:editor_ids, preselection, {}, {
class: "selectize",
multiple: true,
data: {
ajax: true,
filled: false,
model: "user",
placeholder: t("basics.enter_two_letters"),
no_results: t("basics.no_results"),
modal: true
}
})
else
form.select(:editor_ids, editors_preselection(lecture), {},
class: "selectize",
multiple: true,
"data-cy": "lecture-editors-select")
end
end
end
Loading

0 comments on commit 7ba50e8

Please sign in to comment.