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] 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' %> +