From 9d8954bab49721b5672a142f5386d3b82993cd0d Mon Sep 17 00:00:00 2001 From: Splines <37160523+Splines@users.noreply.github.com> Date: Tue, 12 Mar 2024 19:20:49 +0100 Subject: [PATCH 1/4] Disable the user cleaner (#599) * Kneecap the user cleaner * Fix typo --- app/models/user_cleaner.rb | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/app/models/user_cleaner.rb b/app/models/user_cleaner.rb index 877760420..a25292406 100644 --- a/app/models/user_cleaner.rb +++ b/app/models/user_cleaner.rb @@ -89,11 +89,11 @@ def send_hashes end def delete_ghosts - @hash_dict.each do |mail, hash| - u = User.find_by(email: mail, ghost_hash: hash) - move_mail(@email_dict[mail]) if u.present? && @email_dict.present? - u.destroy! if u&.generic? - end + # @hash_dict.each do |mail, hash| + # u = User.find_by(email: mail, ghost_hash: hash) + # move_mail(@email_dict[mail]) if u.present? && @email_dict.present? + # u.destroy! if u&.generic? + # end end def move_mail(message_ids, attempt = 0) @@ -111,14 +111,15 @@ def move_mail(message_ids, attempt = 0) end def clean! - login - search_emails_and_hashes - return if @email_dict.blank? - - send_hashes - sleep(10) - search_emails_and_hashes - delete_ghosts - logout + # TODO: Implement new user cleaner logic + # login + # search_emails_and_hashes + # return if @email_dict.blank? + + # send_hashes + # sleep(10) + # search_emails_and_hashes + # delete_ghosts + # logout end end From 7da7d7e27e4799a2604fc146c05a88d87504ed09 Mon Sep 17 00:00:00 2001 From: Splines <37160523+Splines@users.noreply.github.com> Date: Wed, 13 Mar 2024 00:20:43 +0100 Subject: [PATCH 2/4] Raise key error if env variable is `nil` & update `docker.env` (#597) * Replace `ENV.fetch(..., nil)` by `ENV.fetch(...)` This means that if the env variable is not set, we will encounter an exception which is good as otherwise we'd just ignore that exception even though it might result in serious repercussions. Regex used for matching (in VSCode): ENV.fetch\(".*",\snil\) * Sync `docker.env` with actual file on server * Replace `ENV[]` by `ENV.fetch(...)` whenever applicable Sometimes `ENV[]` is used in a statement where we check if the env variable is present or not. In this case, we did not change it to `ENV.fetch(...)`. * Fix missing quotes around env variable Error we got during deployment: docker.env: line 24: Users/mampf: No such file or directory * Source dummy `docker.env` file (workaround) See the comment in the code for an in-depth explanation for this workaround. * Replace `<>` syntax in env file to allow sourcing it * Copy dummy docker file into container * Use automatic export of env variables by setting `set -o allexport` (or equivalently: `set -a`). * Use new env syntax for registration limit check * Fall back to `nil` as was before for `RAILS_CACHE_ID` --- app/controllers/erdbeere_controller.rb | 12 +++---- app/controllers/lectures_controller.rb | 4 +-- app/controllers/registrations_controller.rb | 10 +++--- app/helpers/application_helper.rb | 4 +-- .../exception_handler/exception_mailer.rb | 2 +- app/models/user_cleaner.rb | 10 +++--- app/views/devise/registrations/new.html.erb | 6 ++-- .../mailers/new_exception.html.erb | 2 +- app/workers/interaction_saver.rb | 4 +-- config/application.rb | 2 +- config/environments/production.rb | 8 ++--- config/initializers/default_setting.rb | 12 +++---- config/initializers/devise.rb | 2 +- config/initializers/thredded.rb | 2 +- docker/production/Dockerfile | 33 ++++++++++++++++++- docker/production/docker.env | 23 +++++++++---- lib/tasks/db.rake | 8 ++--- 17 files changed, 92 insertions(+), 52 deletions(-) diff --git a/app/controllers/erdbeere_controller.rb b/app/controllers/erdbeere_controller.rb index 23c78601e..f47ec1daa 100644 --- a/app/controllers/erdbeere_controller.rb +++ b/app/controllers/erdbeere_controller.rb @@ -8,7 +8,7 @@ def current_ability end def show_example - response = Faraday.get(ENV.fetch("ERDBEERE_API", nil) + "/examples/#{params[:id]}") + response = Faraday.get(ENV.fetch("ERDBEERE_API") + "/examples/#{params[:id]}") @content = if response.status == 200 JSON.parse(response.body)["embedded_html"] else @@ -17,7 +17,7 @@ def show_example end def show_property - response = Faraday.get(ENV.fetch("ERDBEERE_API", nil) + "/properties/#{params[:id]}") + response = Faraday.get(ENV.fetch("ERDBEERE_API") + "/properties/#{params[:id]}") @content = if response.status == 200 JSON.parse(response.body)["embedded_html"] @@ -28,7 +28,7 @@ def show_property def show_structure params[:id] - response = Faraday.get(ENV.fetch("ERDBEERE_API", nil) + "/structures/#{params[:id]}") + response = Faraday.get(ENV.fetch("ERDBEERE_API") + "/structures/#{params[:id]}") @content = if response.status == 200 JSON.parse(response.body)["embedded_html"] else @@ -51,7 +51,7 @@ def cancel_edit_tags def display_info @id = params[:id] @sort = params[:sort] - response = Faraday.get(ENV.fetch("ERDBEERE_API", nil) + + response = Faraday.get(ENV.fetch("ERDBEERE_API") + "/#{@sort.downcase.pluralize}/#{@id}/view_info") @content = JSON.parse(response.body) if response.status != 200 @@ -87,7 +87,7 @@ def update_tags end def fill_realizations_select - response = Faraday.get("#{ENV.fetch("ERDBEERE_API", nil)}/structures/") + response = Faraday.get("#{ENV.fetch("ERDBEERE_API")}/structures/") @tag = Tag.find_by(id: params[:id]) hash = JSON.parse(response.body) @structures = hash["data"].map do |d| @@ -102,7 +102,7 @@ def fill_realizations_select end def find_example - response = Faraday.get("#{ENV.fetch("ERDBEERE_API", nil)}/find?#{find_params.to_query}") + response = Faraday.get("#{ENV.fetch("ERDBEERE_API")}/find?#{find_params.to_query}") @content = if response.status == 200 JSON.parse(response.body)["embedded_html"] else diff --git a/app/controllers/lectures_controller.rb b/app/controllers/lectures_controller.rb index d108fbbc0..a7196ea79 100644 --- a/app/controllers/lectures_controller.rb +++ b/app/controllers/lectures_controller.rb @@ -217,7 +217,7 @@ def edit_structures def search_examples if @lecture.structure_ids.any? - response = Faraday.get("#{ENV.fetch("ERDBEERE_API", nil)}/search") + response = Faraday.get("#{ENV.fetch("ERDBEERE_API")}/search") @form = JSON.parse(response.body)["embedded_html"] # rubocop:disable Style/StringConcatenation @form.gsub!("token_placeholder", @@ -402,7 +402,7 @@ def eager_load_stuff def set_erdbeere_data @structure_ids = @lecture.structure_ids - response = Faraday.get("#{ENV.fetch("ERDBEERE_API", nil)}/structures") + response = Faraday.get("#{ENV.fetch("ERDBEERE_API")}/structures") response_hash = if response.status == 200 JSON.parse(response.body) else diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index bc9152b78..939f2d56b 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -9,12 +9,12 @@ def verify_captcha return true unless ENV["USE_CAPTCHA_SERVICE"] begin - uri = URI.parse(ENV.fetch("CAPTCHA_VERIFY_URL", nil)) + uri = URI.parse(ENV.fetch("CAPTCHA_VERIFY_URL")) data = { message: params["frc-captcha-solution"], - application_token: ENV.fetch("CAPTCHA_APPLICATION_TOKEN", nil) } + application_token: ENV.fetch("CAPTCHA_APPLICATION_TOKEN") } header = { "Content-Type": "text/json" } http = Net::HTTP.new(uri.host, uri.port) - http.use_ssl = true if ENV["CAPTCHA_VERIFY_URL"].include?("https") + http.use_ssl = true if ENV.fetch("CAPTCHA_VERIFY_URL").include?("https") request = Net::HTTP::Post.new(uri.request_uri, header) request.body = data.to_json @@ -70,9 +70,9 @@ def after_sign_up_path_for(_resource) private def check_registration_limit - timeframe = ((ENV["MAMPF_REGISTRATION_TIMEFRAME"] || 15).to_i.minutes.ago..) + timeframe = (ENV.fetch("MAMPF_REGISTRATION_TIMEFRAME", 15).to_i.minutes.ago..) num_new_registrations = User.where(confirmed_at: nil, created_at: timeframe).count - max_registrations = (ENV["MAMPF_MAX_REGISTRATION_PER_TIMEFRAME"] || 40).to_i + max_registrations = ENV.fetch("MAMPF_MAX_REGISTRATION_PER_TIMEFRAME", 40).to_i return if num_new_registrations <= max_registrations # Current number of new registrations is too high diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 50f6b333b..f267191e3 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -16,7 +16,7 @@ def current_lecture def host if Rails.env.production? # rubocop:disable Style/StringConcatenation - ENV.fetch("MEDIA_SERVER", nil) + "/" + ENV.fetch("INSTANCE_NAME", nil) + ENV.fetch("MEDIA_SERVER") + "/" + ENV.fetch("INSTANCE_NAME") # rubocop:enable Style/StringConcatenation else "" @@ -29,7 +29,7 @@ def host # the actual media server. # This is used for the download buttons for videos and manuscripts. def download_host - Rails.env.production? ? ENV.fetch("DOWNLOAD_LOCATION", nil) : "" + Rails.env.production? ? ENV.fetch("DOWNLOAD_LOCATION") : "" end # Returns the full title on a per-page basis. diff --git a/app/mailers/exception_handler/exception_mailer.rb b/app/mailers/exception_handler/exception_mailer.rb index 84db08e96..f56a25431 100644 --- a/app/mailers/exception_handler/exception_mailer.rb +++ b/app/mailers/exception_handler/exception_mailer.rb @@ -5,7 +5,7 @@ class ExceptionMailer < ApplicationMailer # Defaults default subject: I18n.t("exception.exception", - host: ENV.fetch("URL_HOST", nil)) + host: ENV.fetch("URL_HOST")) default from: ExceptionHandler.config.email default template_path: "exception_handler/mailers" # => http://stackoverflow.com/a/18579046/1143732 diff --git a/app/models/user_cleaner.rb b/app/models/user_cleaner.rb index a25292406..8a5763e53 100644 --- a/app/models/user_cleaner.rb +++ b/app/models/user_cleaner.rb @@ -3,9 +3,9 @@ class UserCleaner attr_accessor :imap, :email_dict, :hash_dict def login - @imap = Net::IMAP.new(ENV.fetch("IMAPSERVER", nil), port: 993, ssl: true) - @imap.authenticate("LOGIN", ENV.fetch("PROJECT_EMAIL_USERNAME", nil), - ENV.fetch("PROJECT_EMAIL_PASSWORD", nil)) + @imap = Net::IMAP.new(ENV.fetch("IMAPSERVER"), port: 993, ssl: true) + @imap.authenticate("LOGIN", ENV.fetch("PROJECT_EMAIL_USERNAME"), + ENV.fetch("PROJECT_EMAIL_PASSWORD")) end def logout @@ -15,7 +15,7 @@ def logout def search_emails_and_hashes @email_dict = {} @hash_dict = {} - @imap.examine(ENV.fetch("PROJECT_EMAIL_MAILBOX", nil)) + @imap.examine(ENV.fetch("PROJECT_EMAIL_MAILBOX")) # Mails containing multiple email addresses (Subject: "Undelivered Mail Returned to Sender") @imap.search(["SUBJECT", "Undelivered Mail Returned to Sender"]).each do |message_id| @@ -103,7 +103,7 @@ def move_mail(message_ids, attempt = 0) return if attempt > 3 begin - @imap.examine(ENV.fetch("PROJECT_EMAIL_MAILBOX", nil)) + @imap.examine(ENV.fetch("PROJECT_EMAIL_MAILBOX")) @imap.move(message_ids, "Other Users/mampf/handled_bounces") rescue Net::IMAP::BadResponseError move_mail(message_ids, attempt + 1) diff --git a/app/views/devise/registrations/new.html.erb b/app/views/devise/registrations/new.html.erb index 2cb17d595..2e47ea0f0 100644 --- a/app/views/devise/registrations/new.html.erb +++ b/app/views/devise/registrations/new.html.erb @@ -33,13 +33,13 @@ target: :_blank)), class: "d-inline", for: "dsgvo-consent" %> <%= f.hidden_field :locale, value: I18n.locale %> - <% if ENV['USE_CAPTCHA_SERVICE']%> + <% if ENV["USE_CAPTCHA_SERVICE"]%>

-

+

<%end %>
- <%= f.submit t('.sign_up'), class: 'btn btn-primary', id: 'register-user', disabled:ENV['USE_CAPTCHA_SERVICE'] %> + <%= f.submit t('.sign_up'), class: 'btn btn-primary', id: 'register-user', disabled:ENV.fetch("USE_CAPTCHA_SERVICE") %>
<% end %> diff --git a/app/views/exception_handler/mailers/new_exception.html.erb b/app/views/exception_handler/mailers/new_exception.html.erb index 93b3b9b89..0ebfbbe16 100644 --- a/app/views/exception_handler/mailers/new_exception.html.erb +++ b/app/views/exception_handler/mailers/new_exception.html.erb @@ -1,5 +1,5 @@ <%= t('exception.exception_report', - host: ENV['URL_HOST']) %> + host: ENV.fetch("URL_HOST")) %> <%= @exception.response %> (<%= @exception.status %>) diff --git a/app/workers/interaction_saver.rb b/app/workers/interaction_saver.rb index f6c95faf1..344d53830 100644 --- a/app/workers/interaction_saver.rb +++ b/app/workers/interaction_saver.rb @@ -2,8 +2,8 @@ class InteractionSaver include Sidekiq::Worker def perform(session_id, full_path, referrer, study_participant) - referrer_url = if referrer.to_s.include?(ENV["URL_HOST"]) - referrer.to_s.remove(ENV["URL_HOST"]) + referrer_url = if referrer.to_s.include?(ENV.fetch("URL_HOST")) + referrer.to_s.remove(ENV.fetch("URL_HOST")) .remove("https://").remove("http://") end Interaction.create(session_id: Digest::SHA2.hexdigest(session_id).first(10), diff --git a/config/application.rb b/config/application.rb index 15040fd01..b5e4e0f1e 100644 --- a/config/application.rb +++ b/config/application.rb @@ -24,7 +24,7 @@ class Application < Rails::Application # the framework and any gems in your application. config.exception_handler = { # sends exception emails to a listed email (string // "you@email.com") - email: ENV.fetch("ERROR_EMAIL", nil), + email: ENV.fetch("ERROR_EMAIL"), # All keys interpolated as strings, so you can use # symbols, strings or integers where necessary diff --git a/config/environments/production.rb b/config/environments/production.rb index 805b5c66e..ec2a4e303 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -55,13 +55,13 @@ config.log_tags = [:request_id] # Use a different cache store in production. - config.cache_store = :mem_cache_store, ENV.fetch("MEMCACHED_SERVER", nil) + config.cache_store = :mem_cache_store, ENV.fetch("MEMCACHED_SERVER") # Use a real queuing backend for Active Job (and separate queues per environment) # config.active_job.queue_adapter = :resque # config.active_job.queue_name_prefix = "mampf_#{Rails.env}" config.action_mailer.perform_caching = false - config.action_mailer.default_url_options = { protocol: "https", host: ENV.fetch("URL_HOST", nil) } + config.action_mailer.default_url_options = { protocol: "https", host: ENV.fetch("URL_HOST") } config.action_mailer.delivery_method = :smtp config.action_mailer.perform_deliveries = true @@ -69,9 +69,9 @@ config.action_mailer.default(charset: "utf-8") config.action_mailer.smtp_settings = { - address: ENV.fetch("MAILSERVER", nil), + address: ENV.fetch("MAILSERVER"), port: 25, - domain: ENV.fetch("MAILSERVER", nil) + domain: ENV.fetch("MAILSERVER") } # Ignore bad email addresses and do not raise email delivery errors. diff --git a/config/initializers/default_setting.rb b/config/initializers/default_setting.rb index bdc25a5da..49d0ea455 100644 --- a/config/initializers/default_setting.rb +++ b/config/initializers/default_setting.rb @@ -1,10 +1,10 @@ class DefaultSetting - ERDBEERE_LINK = ENV.fetch("ERDBEERE_SERVER", nil) - MUESLI_LINK = ENV.fetch("MUESLI_SERVER", nil) - PROJECT_EMAIL = ENV.fetch("PROJECT_EMAIL", nil) - PROJECT_NOTIFICATION_EMAIL = ENV.fetch("PROJECT_NOTIFICATION_EMAIL", nil) - BLOG_LINK = ENV.fetch("BLOG", nil) - URL_HOST_SHORT = ENV.fetch("URL_HOST_SHORT", nil) + ERDBEERE_LINK = ENV.fetch("ERDBEERE_SERVER") + MUESLI_LINK = ENV.fetch("MUESLI_SERVER") + PROJECT_EMAIL = ENV.fetch("PROJECT_EMAIL") + PROJECT_NOTIFICATION_EMAIL = ENV.fetch("PROJECT_NOTIFICATION_EMAIL") + BLOG_LINK = ENV.fetch("BLOG") + URL_HOST_SHORT = ENV.fetch("URL_HOST_SHORT") RESEARCHGATE_LINK = "https://www.researchgate.net/project/MaMpf-Mathematische-Medienplattform".freeze TOUR_LINK = "https://mampf.blog/ueber-mampf/".freeze RESOURCES_LINK = "https://mampf.blog/ressourcen-fur-editorinnen/".freeze diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index c3b179a48..d900ea888 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -12,7 +12,7 @@ # note that it will be overwritten if you use your own mailer class # with default "from" parameter. config.mailer_sender = if Rails.env.production? - ENV.fetch("FROM_ADDRESS", nil) + ENV.fetch("FROM_ADDRESS") else "please-change-me-at-config-initializers-devise@example.com" end diff --git a/config/initializers/thredded.rb b/config/initializers/thredded.rb index 75f4539e3..30b0ae4a3 100644 --- a/config/initializers/thredded.rb +++ b/config/initializers/thredded.rb @@ -190,4 +190,4 @@ # # add in (must install separate gem (under development) as well): # Thredded.notifiers = [Thredded::EmailNotifier.new, -# Thredded::PushoverNotifier.new(ENV.fetch("PUSHOVER_APP_ID", nil))] +# Thredded::PushoverNotifier.new(ENV.fetch("PUSHOVER_APP_ID"))] diff --git a/docker/production/Dockerfile b/docker/production/Dockerfile index d7dd28f20..c1856e773 100644 --- a/docker/production/Dockerfile +++ b/docker/production/Dockerfile @@ -81,5 +81,36 @@ RUN bundle install RUN yarn install --production=true COPY --chown=app:app . /usr/src/app + +# The command ". ./docker-dummy.env" will source our dummy docker env file. +# So why do we need this? +# +# Well, (deeply inhales), Rails needs to boot entirely to run the +# `assets:precompile` task. Therefore, it also needs to access the env variables +# to correctly start the initializers. +# +# However (after a long ime researching), docker compose does not seem to offer +# an easy solution to have an env file from the host machine available during +# the build step (Dockerfile) and not just during the run time of the container. +# Note that the env file is indeed available on our host, just not in the build +# context, the latter being the MaMpf github repo that docker compose pulls from. +# +# Even with volumes and bind mounts it's not working properly ("file not found"). +# In the end, we found a solution that suggests to use the new docker buildkit +# to allow for multiple build contexts. Yet, we explicitly set DOCKER_BUILDKIT=0 +# to use the old buildkit since the new one always gives a moby-related ssh error. +# And even if this worked, it is not entirely clear if this is even working +# with docker compose or just with docker (sigh). +# +# That's why, in the end, we decided to leverage our already-existing dummy env +# file and source it here in the Dockerfile just to have the precompile task run +# successfully (this task doesn't even rely on the actual values, so despite +# being a hack, it should be fine). +# +# I've written down more details in this question on StackOverflow: +# https://stackoverflow.com/q/78098380/ +COPY ./docker/production/docker.env ./docker-dummy.env + RUN cp -r $(bundle info --path sidekiq)/web/assets /usr/src/app/public/sidekiq && \ - SECRET_KEY_BASE="$(bundle exec rails secret)" DB_ADAPTER=nulldb bundle exec rails assets:precompile + set -o allexport && . ./docker-dummy.env && set +o allexport && \ + SECRET_KEY_BASE="$(bundle exec rails secret)" DB_ADAPTER=nulldb bundle exec rails assets:precompile diff --git a/docker/production/docker.env b/docker/production/docker.env index 8070eb0d9..9704dd428 100644 --- a/docker/production/docker.env +++ b/docker/production/docker.env @@ -1,4 +1,5 @@ -GIT_BRANCH=production +# Instance variables +GIT_BRANCH=main COMPOSE_PROJECT_NAME=mampf INSTANCE_NAME=mampf @@ -14,15 +15,23 @@ MEMCACHED_SERVER=cache FROM_ADDRESS=mampf@mathi.uni-heidelberg.de MAILSERVER=mail.mathi.uni-heidelberg.de PROJECT_EMAIL=mampf@mathi.uni-heidelberg.de -ERROR_EMAIL=mampf-error@mathi.uni-heidelberg.de +PROJECT_NOTIFICATION_EMAIL=notificationmail MAILID_DOMAIN=mathi.uni-heidelberg.de +ERROR_EMAIL=mampf-error@mathi.uni-heidelberg.de IMAPSERVER=mail.mathi.uni-heidelberg.de PROJECT_EMAIL_USERNAME=creativeusername PROJECT_EMAIL_PASSWORD=secretsecret -PROJECT_EMAIL_MAILBOX=Other Users/mampf +PROJECT_EMAIL_MAILBOX="Other Users/mampf" # Due to CORS constraints, some urls are proxied to the media server DOWNLOAD_LOCATION=https://mampf.mathi.uni-heidelberg.de/mediaforward +REWRITE_ENABLED=1 + +# Captcha Server +USE_CAPTCHA_SERVICE=1 +CAPTCHA_VERIFY_URL=https://captcha2go.mathi.uni-heidelberg.de/v1/verify +CAPTCHA_PUZZLE_URL=https://captcha2go.mathi.uni-heidelberg.de/v1/puzzle +CAPTCHA_APPLICATION_TOKEN=secret # Upload folder MEDIA_PATH=/private/media @@ -35,14 +44,14 @@ PRODUCTION_DATABASE_INTERACTIONS=mampf_interactions PRODUCTION_DATABASE_HOST=db PRODUCTION_DATABASE_USERNAME=mampf PRODUCTION_DATABASE_PASSWORD=supersecret -PRODUCTION_DATABASE_PORT=5432 -PRODUCTION_DATABASE_URL='postgresql://mampf:supersecret@db:5432/mampf' +PRODUCTION_DATABASE_PORT=port +PRODUCTION_DATABASE_URL='postgresql://mampf:supersecret@db:port/mampf' # Rails configuration # change RAILS_ENV to production for a production deployment RAILS_ENV=production -RAILS_MASTER_KEY= -SECRET_KEY_BASE= +RAILS_MASTER_KEY=secret +SECRET_KEY_BASE=secret URL_HOST=mampf.mathi.uni-heidelberg.de URL_HOST_SHORT=http://mampf.media diff --git a/lib/tasks/db.rake b/lib/tasks/db.rake index d7aa4dd01..067cd8ac2 100644 --- a/lib/tasks/db.rake +++ b/lib/tasks/db.rake @@ -33,7 +33,7 @@ namespace :db do desc "Dumps the database to backups" task dump: :environment do - dump_fmt = ensure_format(ENV.fetch("format", nil)) + dump_fmt = ensure_format(ENV.fetch("format")) dump_sfx = suffix_for_format(dump_fmt) backup_dir = backup_directory(Rails.env, create: true) full_path = nil @@ -56,10 +56,10 @@ namespace :db do namespace :dump do desc "Dumps a specific table to backups" task table: :environment do - table_name = ENV.fetch("table", nil) + table_name = ENV.fetch("table") if table_name.present? - dump_fmt = ensure_format(ENV.fetch("format", nil)) + dump_fmt = ensure_format(ENV.fetch("format")) dump_sfx = suffix_for_format(dump_fmt) backup_dir = backup_directory(Rails.env, create: true) full_path = nil @@ -92,7 +92,7 @@ namespace :db do desc "Restores the database from a backup using PATTERN" task restore: :environment do - pattern = ENV.fetch("pattern", nil) + pattern = ENV.fetch("pattern") if pattern.present? file = nil From 469bb11377568b68e11d4b850457dc59f5d3f4b0 Mon Sep 17 00:00:00 2001 From: Splines <37160523+Splines@users.noreply.github.com> Date: Tue, 19 Mar 2024 12:10:20 +0100 Subject: [PATCH 3/4] Authenticate mails sent by MaMpf (#590) * Authenticate mails sent by MaMpf * Fix mails comment * Add `nil` env fallback due to #542 (this one was removed again in merge commit 4d066c3) --- config/environments/production.rb | 3 ++- docker/production/docker.env | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/config/environments/production.rb b/config/environments/production.rb index ec2a4e303..ae9c17db1 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -71,7 +71,8 @@ config.action_mailer.smtp_settings = { address: ENV.fetch("MAILSERVER"), port: 25, - domain: ENV.fetch("MAILSERVER") + user_name: ENV.fetch("MAMPF_EMAIL_USERNAME"), + password: ENV.fetch("MAMPF_EMAIL_PASSWORD") } # Ignore bad email addresses and do not raise email delivery errors. diff --git a/docker/production/docker.env b/docker/production/docker.env index 9704dd428..11523738e 100644 --- a/docker/production/docker.env +++ b/docker/production/docker.env @@ -11,7 +11,7 @@ MUESLI_SERVER=https://muesli.mathi.uni-heidelberg.de ERDBEERE_API=https://erdbeere.mathi.uni-heidelberg.de/api/v1 MEMCACHED_SERVER=cache -# Email is send using a mailserver without authentication. Specify how to connect here +# Email FROM_ADDRESS=mampf@mathi.uni-heidelberg.de MAILSERVER=mail.mathi.uni-heidelberg.de PROJECT_EMAIL=mampf@mathi.uni-heidelberg.de @@ -22,6 +22,8 @@ IMAPSERVER=mail.mathi.uni-heidelberg.de PROJECT_EMAIL_USERNAME=creativeusername PROJECT_EMAIL_PASSWORD=secretsecret PROJECT_EMAIL_MAILBOX="Other Users/mampf" +MAMPF_EMAIL_USERNAME=secret +MAMPF_EMAIL_PASSWORD=secret # Due to CORS constraints, some urls are proxied to the media server DOWNLOAD_LOCATION=https://mampf.mathi.uni-heidelberg.de/mediaforward From 4e031a6adefa80a48df00e2f4d315173138a5c79 Mon Sep 17 00:00:00 2001 From: Splines <37160523+Splines@users.noreply.github.com> Date: Wed, 20 Mar 2024 23:25:53 +0100 Subject: [PATCH 4/4] Add feedback button (#527) Due to a forced push of the dev branch, the following list might unfortunately include items from other branches as well. * Init Feedback model * Add Feedback modal view and corresponding controller First working version, of course still a lot to improve from here. * Migrate feedback form to Bootstrap v5 * Add basic styling to Feedback form * Add "allow contact via mail" checkbox A new column was added to the Feedbacks schema. Note that we did not create a new migration as this is a PR which should only contain one migration, namely the one for the creation of the whol Feedback table. * Toggle "allow email contact" by default * Improve submit button handler (outsource to function) * Init feedback mailer Right now just for ourselves, so that we get a plaintext mail with the feedback of a user. Env variables were adjusted accordingly, but need to be set manually in the production environment! * Adjust feedback mail in views * Implement success/error flow with toast messages * Add missing database field "can_contact" * Add internationalization to feedback error/success * Lint some files * Set feedback text field as required with min 10 chars * Add "optional" to title in email * Adjust spacing around feedback button * Internationalize tooltip * Delete console log * Add comment describing hidden submit button handler * Delete default test specs * Add proper validation for Feedback body Alongside this, also made sure that we use a custom client-side validation message when input is too short (under 10 chars long). This allows us to use the language the user has selected in MaMpf instead of the browser language. * Default `can_contact` to false in backend * Update bootstrap to v5.3.1 command used: bundle update bootstrap bundle update bootstrap --conservative did not work, as docker containers did not start again due to dependency errors * Revert "Update bootstrap to v5.3.1" in favor of PR #537 This reverts commit 5cd1af25820f0eea73a98a99073634ef77daa75a. * Submit form via Ctrl + Enter when modal is opened * Remove default nil value from ENV.fetch() * Revert "Remove default nil value from ENV.fetch()" This reverts commit 696a395cce1bb24d1018e84479a85cc0a66d77ba. * Rename button to 'Send' (not 'Save') * Check if should register feedback event handlers * Make feedback button ID more specific * Fix line wrapping (code style) * Use delete on cascade to be able to delete a user even if he/she has sent some feedback * Move Send button before Cancel button * Replace "on delete cascade" with "dependent destroy" * Add cypress rules to ESLint & ignore some patterns * Allow usage of tempusDominus global variable * Ignore JS files with Sprocket syntax * Further improve rules, e.g. allow common globals * Ignore sprocket syntax in cable.js * Autofix all `.js` and `.js.erb` files Command used: `yarn run eslint --fix .` Still 47 problems (27 errors, 20 warnings) after this. * Fix variables in turbolink fix * Prepend unused variables with "_" * Get rid of unused widget variable * Fix specs comment tab alignment * Warn about too long GitHub commit messages (#586) * Fix comment status (#585) * Reapply first fix for Reader/Media See discussion on #574 for further details. Previous PR for this was #576, closed in favor of this one as this directly branches off the new "dev" branch. * Correctly show latest post (might be current_user's comment) * Fix update of unread comments logic in comments controller * Fix update icon logic and latest post comment * Simplify latest comment logic * Improve code comments * Further improve comments * Fix wording in comment * Fix construction of media array & use `.blank?` instead of `.empty?` * Migrate `unread_comments` flag (fix inconsistencies) (#587) * Add dummy migration * Implement migration for unread comment flag * Remove unnecessary comment * Declare migration as not idempotent * Use array.length instead of counting * Throw error to prevent revert of migration * Fix severe flaws in unread comments migration * Simplify Reader retrieval * Use the more explicit `.nil?` method * Update migration date * Fix annoying bug: don't use `.select!` but `.select` * Polish migration e.g. update comment, more suitable name for the method etc. * Rename method according to #585 * Use `warn` log level for migration (#588) * Fix linting in feedback.js * Fix RuboCop errors * Fix remaining ESLint errors * Update timestamp of feedback migration * Add missing Feedback email to prod docker.env * Remove unnecessary Feedback env variables * Add validation message for empty body * Change `const` to `var` to avoid "redefined" errors * Update timestamp of feedback migration (again) --- app/assets/javascripts/feedback.js | 74 +++++++++++++++++++ app/assets/javascripts/lectures.coffee | 2 + app/assets/stylesheets/application.scss | 4 + app/assets/stylesheets/feedback.scss | 11 +++ app/controllers/feedbacks_controller.rb | 21 ++++++ app/mailers/feedback_mailer.rb | 15 ++++ app/models/feedback.rb | 10 +++ app/models/user.rb | 3 + .../new_user_feedback_email.text.erb | 13 ++++ app/views/feedbacks/_feedback.html.erb | 44 +++++++++++ app/views/feedbacks/_feedback_button.html.erb | 8 ++ app/views/feedbacks/_feedback_form.html.erb | 63 ++++++++++++++++ app/views/feedbacks/create.js.erb | 7 ++ .../shared/_dropdown_notifications.html.erb | 4 +- app/views/shared/_footer.html.erb | 2 +- app/views/shared/_navbar.html.erb | 7 ++ config/initializers/default_setting.rb | 1 + config/locales/de.yml | 30 ++++++++ config/locales/en.yml | 29 ++++++++ config/routes.rb | 3 + db/migrate/20240319130000_create_feedbacks.rb | 12 +++ db/schema.rb | 13 +++- docker/development/docker-compose.yml | 1 + docker/production/docker.env | 1 + .../docker-compose.local.yml | 1 + docker/run_cypress_tests/docker-compose.yml | 1 + 26 files changed, 376 insertions(+), 4 deletions(-) create mode 100644 app/assets/javascripts/feedback.js create mode 100644 app/assets/stylesheets/feedback.scss create mode 100644 app/controllers/feedbacks_controller.rb create mode 100644 app/mailers/feedback_mailer.rb create mode 100644 app/models/feedback.rb create mode 100644 app/views/feedback_mailer/new_user_feedback_email.text.erb create mode 100644 app/views/feedbacks/_feedback.html.erb create mode 100644 app/views/feedbacks/_feedback_button.html.erb create mode 100644 app/views/feedbacks/_feedback_form.html.erb create mode 100644 app/views/feedbacks/create.js.erb create mode 100644 db/migrate/20240319130000_create_feedbacks.rb diff --git a/app/assets/javascripts/feedback.js b/app/assets/javascripts/feedback.js new file mode 100644 index 000000000..7d9ab3772 --- /dev/null +++ b/app/assets/javascripts/feedback.js @@ -0,0 +1,74 @@ +$(document).on("turbolinks:load", () => { + if (!shouldRegisterFeedback()) { + return; + } + registerToasts(); + registerSubmitButtonHandler(); + registerFeedbackBodyValidator(); +}); + +var SUBMIT_FEEDBACK_ID = "#submit-feedback"; + +var TOAST_OPTIONS = { + animation: true, + autohide: true, + delay: 6000, // autohide after ... milliseconds +}; + +function shouldRegisterFeedback() { + return $(SUBMIT_FEEDBACK_ID).length > 0; +} + +function registerToasts() { + const toastElements = document.querySelectorAll(".toast"); + [...toastElements].map((toast) => { + new bootstrap.Toast(toast, TOAST_OPTIONS); + }); +} + +function registerSubmitButtonHandler() { + // Invoke the hidden submit button inside the actual Rails form + $("#submit-feedback-form-btn-outside").click(() => { + submitFeedback(); + }); + + // Submit form by pressing Ctrl + Enter + document.addEventListener("keydown", (event) => { + const isModalOpen = $(SUBMIT_FEEDBACK_ID).is(":visible"); + if (isModalOpen && event.ctrlKey && event.key == "Enter") { + submitFeedback(); + } + }); +} + +function registerFeedbackBodyValidator() { + const feedbackBody = document.getElementById("feedback_feedback"); + feedbackBody.addEventListener("input", () => { + validateFeedback(); + }); +} + +function validateFeedback() { + const feedbackBody = document.getElementById("feedback_feedback"); + const validityState = feedbackBody.validity; + if (validityState.tooShort) { + const tooShortMessage = feedbackBody.dataset.tooShortMessage; + feedbackBody.setCustomValidity(tooShortMessage); + } + else if (validityState.valueMissing) { + const valueMissingMessage = feedbackBody.dataset.valueMissingMessage; + feedbackBody.setCustomValidity(valueMissingMessage); + } + else { + // render input valid, so that form will submit + feedbackBody.setCustomValidity(""); + } + + feedbackBody.reportValidity(); +} + +function submitFeedback() { + const submitButton = $("#submit-feedback-form-btn"); + validateFeedback(); + submitButton.click(); +} diff --git a/app/assets/javascripts/lectures.coffee b/app/assets/javascripts/lectures.coffee index 15dfacca0..5e47d4fea 100644 --- a/app/assets/javascripts/lectures.coffee +++ b/app/assets/javascripts/lectures.coffee @@ -214,6 +214,7 @@ $(document).on 'turbolinks:load', -> $('#secondnav').show() $('#lecturesDropdown').appendTo($('#secondnav')) $('#notificationDropdown').appendTo($('#secondnav')) + $('#feedback-btn').appendTo($('#secondnav')) $('#searchField').appendTo($('#secondnav')) $('#second-admin-nav').show() $('#adminDetails').appendTo($('#second-admin-nav')) @@ -236,6 +237,7 @@ $(document).on 'turbolinks:load', -> $('#secondnav').hide() $('#lecturesDropdown').appendTo($('#firstnav')) $('#notificationDropdown').appendTo($('#firstnav')) + $('#feedback-btn').appendTo($('#firstnav')) $('#searchField').appendTo($('#firstnav')) $('#second-admin-nav').hide() $('#teachableDrop').appendTo($('#first-admin-nav')) diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 35f8c0d78..4fed1ddea 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -302,4 +302,8 @@ a { &:hover { text-decoration: underline; } +} + +.toast { + background-color: white; } \ No newline at end of file diff --git a/app/assets/stylesheets/feedback.scss b/app/assets/stylesheets/feedback.scss new file mode 100644 index 000000000..7f14ad18c --- /dev/null +++ b/app/assets/stylesheets/feedback.scss @@ -0,0 +1,11 @@ +#feedback-btn { + color: white; + + &:focus { + box-shadow: none; + } + + &:hover { + color: #ffc107; + } +} \ No newline at end of file diff --git a/app/controllers/feedbacks_controller.rb b/app/controllers/feedbacks_controller.rb new file mode 100644 index 000000000..9ae31a957 --- /dev/null +++ b/app/controllers/feedbacks_controller.rb @@ -0,0 +1,21 @@ +class FeedbacksController < ApplicationController + authorize_resource except: [:create] + + def create + feedback = Feedback.new(feedback_params) + feedback.user_id = current_user.id + @feedback_success = feedback.save + + if @feedback_success + FeedbackMailer.with(feedback: feedback).new_user_feedback_email.deliver_later + end + + respond_to(&:js) + end + + private + + def feedback_params + params.require(:feedback).permit(:title, :feedback, :can_contact) + end +end diff --git a/app/mailers/feedback_mailer.rb b/app/mailers/feedback_mailer.rb new file mode 100644 index 000000000..152b1378b --- /dev/null +++ b/app/mailers/feedback_mailer.rb @@ -0,0 +1,15 @@ +class FeedbackMailer < ApplicationMailer + default from: DefaultSetting::FEEDBACK_EMAIL + layout false + + # Mail to the MaMpf developers including the new feedback of a user. + def new_user_feedback_email + @feedback = params[:feedback] + reply_to_mail = @feedback.can_contact ? @feedback.user.email : "" + subject = "Feedback: #{@feedback.title}" + mail(to: DefaultSetting::FEEDBACK_EMAIL, + subject: subject, + content_type: "text/plain", + reply_to: reply_to_mail) + end +end diff --git a/app/models/feedback.rb b/app/models/feedback.rb new file mode 100644 index 000000000..2cc0a0c6b --- /dev/null +++ b/app/models/feedback.rb @@ -0,0 +1,10 @@ +# Feedback from users regarding MaMpf itself. +class Feedback < ApplicationRecord + belongs_to :user + + BODY_MIN_LENGTH = 10 + BODY_MAX_LENGTH = 10_000 + validates :feedback, length: { minimum: BODY_MIN_LENGTH, + maximum: BODY_MAX_LENGTH }, + allow_blank: false +end diff --git a/app/models/user.rb b/app/models/user.rb index 40feb6f60..4b612f20e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -78,6 +78,9 @@ class User < ApplicationRecord # a user has a watchlist with watchlist_entries has_many :watchlists, dependent: :destroy + + has_many :feedbacks, dependent: :destroy + include ScreenshotUploader[:image] # if a homepage is given it should at leat be a valid address diff --git a/app/views/feedback_mailer/new_user_feedback_email.text.erb b/app/views/feedback_mailer/new_user_feedback_email.text.erb new file mode 100644 index 000000000..9b580a4a4 --- /dev/null +++ b/app/views/feedback_mailer/new_user_feedback_email.text.erb @@ -0,0 +1,13 @@ +# Title (optional) +<%= @feedback.title %> + +# Feedback +<%= @feedback.feedback %> + + +----- +<% if @feedback.can_contact %> +Reply to this mail to contact the user. +<% else %> +User did not give permission to contact them regarding their feedback, so we cannot reply to this mail. +<% end %> diff --git a/app/views/feedbacks/_feedback.html.erb b/app/views/feedbacks/_feedback.html.erb new file mode 100644 index 000000000..b98f5c8bc --- /dev/null +++ b/app/views/feedbacks/_feedback.html.erb @@ -0,0 +1,44 @@ +<%= stylesheet_link_tag 'feedback' %> + +<%# Main Modal %> + + +<%# Messages toasts %> +
+ <%# Error %> + + + <%# Success %> + + +
diff --git a/app/views/feedbacks/_feedback_button.html.erb b/app/views/feedbacks/_feedback_button.html.erb new file mode 100644 index 000000000..c3d9c02fa --- /dev/null +++ b/app/views/feedbacks/_feedback_button.html.erb @@ -0,0 +1,8 @@ +<%= stylesheet_link_tag 'feedback' %> + +<%# Feedback button %> + diff --git a/app/views/feedbacks/_feedback_form.html.erb b/app/views/feedbacks/_feedback_form.html.erb new file mode 100644 index 000000000..2e0833e3e --- /dev/null +++ b/app/views/feedbacks/_feedback_form.html.erb @@ -0,0 +1,63 @@ +<%= javascript_include_tag :feedback %> + +
+ +

<%= t('feedback.modal_title') %>

+ +
+ +
+ <%= t('feedback.description_html', + github_mampf: link_to('GitHub', 'https://github.com/MaMpf-HD/mampf/issues', {target: '_blank'}), + feedback_mail: mail_to(DefaultSetting::FEEDBACK_EMAIL, t('basics.mail_noun'))) %> + + <%= form_with model: Feedback.new, remote: true do |f| %> + +
+ <%= f.text_field :title, class: 'form-control', placeholder: '_' %> + <%= f.label :title, t('feedback.title') %> +
+ + +
+ <%= f.text_area :feedback, + class: 'form-control', + placeholder: '_', + style: 'height: 200px', + required: '', + minlength: Feedback::BODY_MIN_LENGTH, + maxlength: Feedback::BODY_MAX_LENGTH, + 'data-too-short-message': t('feedback.body_too_short_error', + min_length: Feedback::BODY_MIN_LENGTH), + 'data-value-missing-message': t('feedback.body_missing_error') %> + <%= f.label :feedback, t('feedback.comment') %> +
+ + +
+ <%= f.check_box :can_contact, + checked: 'checked', + class: 'form-check-input' %> + <%= f.label :can_contact, + t('feedback.mail_checkbox', user_mail: @current_user.email), + class: 'form-check-label' %> +
+ + + + <%= f.submit 'Submit', id: 'submit-feedback-form-btn', style: 'display: none;' %> + <% end %> +
+ +
+ + + + + +
diff --git a/app/views/feedbacks/create.js.erb b/app/views/feedbacks/create.js.erb new file mode 100644 index 000000000..1538b24bf --- /dev/null +++ b/app/views/feedbacks/create.js.erb @@ -0,0 +1,7 @@ +<% if @feedback_success %> +$("#submit-feedback").modal("hide"); +$("#submit-feedback").find("form").trigger("reset"); // clear form +$("#toast-successfully-sent").toast("show"); +<% else %> +$("#toast-could-not-send").toast("show"); +<% end %> diff --git a/app/views/shared/_dropdown_notifications.html.erb b/app/views/shared/_dropdown_notifications.html.erb index 2117400a9..5405d3bc0 100644 --- a/app/views/shared/_dropdown_notifications.html.erb +++ b/app/views/shared/_dropdown_notifications.html.erb @@ -1,7 +1,7 @@ <% relevant_notifications = current_user.notifications .sort_by(&:created_at).reverse %> <% if relevant_notifications.present? %> - -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/shared/_footer.html.erb b/app/views/shared/_footer.html.erb index efb373378..501a4b168 100644 --- a/app/views/shared/_footer.html.erb +++ b/app/views/shared/_footer.html.erb @@ -46,7 +46,7 @@ bugs: link_to(t('here'), 'https://github.com/MaMpf-HD/mampf/issues', target: :_blank), - feedback: mail_to(DefaultSetting::PROJECT_EMAIL, nil), + feedback: mail_to(DefaultSetting::FEEDBACK_EMAIL, nil), background_photo_credit: link_to('Craig S. Kaplan', 'https://github.com/isohedral/hatviz', target: :_blank)) %> diff --git a/app/views/shared/_navbar.html.erb b/app/views/shared/_navbar.html.erb index 18cc82648..73e98ebf1 100644 --- a/app/views/shared/_navbar.html.erb +++ b/app/views/shared/_navbar.html.erb @@ -1,5 +1,8 @@ <% I18n.with_locale(current_user.try(:locale)) do %> <%= stylesheet_link_tag 'navbar' %> + +<%= render partial: 'feedbacks/feedback' %> +