diff --git a/app/controllers/feedback_forms_controller.rb b/app/controllers/feedback_forms_controller.rb index 0eaa2d29..8b94ba25 100644 --- a/app/controllers/feedback_forms_controller.rb +++ b/app/controllers/feedback_forms_controller.rb @@ -1,49 +1,22 @@ # frozen_string_literal: true class FeedbackFormsController < ApplicationController - EMAIL_PRESENT_MESSAGE = 'You have filled in a field that makes you appear as a spammer. Please follow the directions for the individual form fields.' - MESSAGE_BLANK_MESSAGE = 'A message is required' - RECAPTCHA_MESSAGE = 'You must pass the reCAPTCHA challenge' - - # The client handles rendering flash in this case, so clear it on the server - # side to prevent it from rendering on the next request. - after_action :discard_flash!, only: :create, if: -> { request.xhr? } - - def new - return render :new_frame if params[:frame] == 'true' - - render :new - end + def new; end def create - if validate + if pass_captcha? FeedbackMailer.submit_feedback(params, request.remote_ip).deliver_now flash[:success] = 'Thank you! Your feedback has been sent.' + else + flash[:error] = 'You must pass the reCAPTCHA challenge' end - respond_to do |format| - format.json do - render json: flash - end - format.html do - redirect_to params[:url] - end - end + redirect_to params[:url] end protected - def validate - errors = [] - errors << MESSAGE_BLANK_MESSAGE if params[:message].blank? - errors << EMAIL_PRESENT_MESSAGE if params[:email_address].present? - errors << RECAPTCHA_MESSAGE if current_user.blank? && !verify_recaptcha - - flash[:error] = errors.join('
') unless errors.empty? - flash[:error].nil? - end - - def discard_flash! - flash.discard + def pass_captcha? + current_user.present? || verify_recaptcha end end diff --git a/app/javascript/application.js b/app/javascript/application.js index bbc06e94..f104703b 100644 --- a/app/javascript/application.js +++ b/app/javascript/application.js @@ -1,5 +1,4 @@ // Configure your import map in config/importmap.rb. Read more: https://github.com/rails/importmap-rails import "controllers" import 'bootstrap'; -import 'feedback_form'; import "@hotwired/turbo-rails" diff --git a/app/javascript/controllers/feedback_controller.js b/app/javascript/controllers/feedback_controller.js new file mode 100644 index 00000000..6e66ab99 --- /dev/null +++ b/app/javascript/controllers/feedback_controller.js @@ -0,0 +1,10 @@ +import { Controller } from '@hotwired/stimulus' + +export default class extends Controller { + static targets = ['form'] + + connect() { + this.formTarget['user_agent'].value = navigator.userAgent + this.formTarget['viewport'].value = `width: ${window.innerWidth} height: ${innerHeight}` + } +} \ No newline at end of file diff --git a/app/javascript/feedback_form.js b/app/javascript/feedback_form.js deleted file mode 100644 index 722cd8b8..00000000 --- a/app/javascript/feedback_form.js +++ /dev/null @@ -1,122 +0,0 @@ -import { Collapse } from 'bootstrap' -import jQuery from 'jquery' -const $ = jQuery - -$(function(){ - // Instantiates plugin for feedback form - - $("#feedback-form").feedbackForm(); -}); - -(function ($, window, document, undefined) { - /* - jQuery plugin that handles some of the feedback form functionality - - Usage: $(selector).feedbackForm(); - - No available options - - This plugin : - - submits an ajax request for the feedback form - - displays alert on response from feedback form - */ - - var pluginName = "feedbackForm"; - - function Plugin(element, options) { - this.element = element; - this.options = $.extend({}, options); - this._name = pluginName; - this.init(); - } - - function isSuccess(response){ - switch(response[0][0]){ - case 'success': - return true; - default: - return false; - } - } - - function renderFlashMessages(response) { - $.each(response, function(i,val){ - var flashHtml = ""; - - // Show the flash message - $('div.flash_messages').html(flashHtml); - }); - } - - function alertClassFrom(flashLevel) { - switch(flashLevel) { - case 'notice': - return 'info'; - case 'alert': - return 'warning'; - case 'error': - return 'danger'; - default: - return flashLevel; - } - } - - Plugin.prototype = { - init: function() { - this.$el = $(this.element); - this.$form = $(this.$el).find('form'); - - // Add listener for form submit - this.submitListener(); - - // Updates reporting from fields for current location - $('span.reporting-from-field').html(location.href); - $('input.reporting-from-field').val(location.href); - - // Focus message textarea when showing collapsible form - $('#feedback-form').on('shown.bs.collapse', function () { - $("textarea#message").focus(); - }); - }, - - submitListener: function () { - const el = this.$el; - - // Serialize and submit form if not on action url - this.$form.each(function(i, form){ - if (location !== form.action){ - $('#user_agent').val(navigator.userAgent); - $('#viewport').val('width:' + window.innerWidth + ' height:' + innerHeight); - $(form).on('submit', function(){ - var valuesToSubmit = $(this).serialize(); - $.ajax({ - url: form.action, - data: valuesToSubmit, - type: 'post' - }).done(function(response){ - if (isSuccess(response)){ - // This is the BS5 way to toggle a collapsible - new Collapse(el); - form.reset(); - } - renderFlashMessages(response); - }); - return false; - }); - - } - }); - } - }; - - // A really lightweight plugin wrapper around the constructor, - // preventing against multiple instantiations - $.fn[pluginName] = function (options) { - return this.each(function () { - if (!$.data(this, "plugin_" + pluginName)) { - $.data(this, "plugin_" + pluginName, - new Plugin(this, options)); - } - }); - }; -})(jQuery, window, document); diff --git a/app/views/feedback_forms/new.html.erb b/app/views/feedback_forms/new.html.erb index 4ac636ca..b4c0f45c 100644 --- a/app/views/feedback_forms/new.html.erb +++ b/app/views/feedback_forms/new.html.erb @@ -1 +1,53 @@ -<%= render 'shared/feedback_forms/form' %> + +
+
+ <%= form_tag feedback_form_path, method: :post, class:"form-horizontal feedback-form", role:"form", data: { turbo: false, controller: 'feedback', feedback_target: 'form' } do %> +
+
+ +
+
+ <%= hidden_field_tag :user_agent %> + <%= hidden_field_tag :viewport %> +
+
+ <%= label_tag(:message, 'Message', class: "col-sm-3 col-form-label text-end") %> +
+ <%= text_area_tag :message, "", rows:"5", class:"form-control", required: true %> +
+
+
+ <%= label_tag(:name, 'Your name', class: "col-sm-3 col-form-label text-end") %> +
+ <%= text_field_tag :name, "", class:"form-control", required: true %> +
+
+
+ <%= label_tag(:to, 'Your email', class: "col-sm-3 col-form-label text-end") %> +
+ <%= email_field_tag :to, "", class:"form-control", required: true %> +
+
+ <% if current_user.blank? %> +
+
+ <%= recaptcha_tags %> + +

(Stanford users can avoid this Captcha by logging in.)

+
+
+ <% end %> +
+
+ + <%= button_tag "Cancel", type: 'button', class: 'btn btn-link', data: { bs_toggle: 'collapse', bs_target: '#feedback-form' }, aria: { expanded: false, controls: 'feedback-form' } %> +
+
+
+ <% end %> +
+
+ diff --git a/app/views/feedback_forms/new_frame.html.erb b/app/views/feedback_forms/new_frame.html.erb deleted file mode 100644 index ab207c97..00000000 --- a/app/views/feedback_forms/new_frame.html.erb +++ /dev/null @@ -1,5 +0,0 @@ - -
- <%= render 'shared/feedback_forms/form' %> -
- diff --git a/app/views/shared/_top_navbar.html.erb b/app/views/shared/_top_navbar.html.erb index d0fb03ae..5ed77e5c 100644 --- a/app/views/shared/_top_navbar.html.erb +++ b/app/views/shared/_top_navbar.html.erb @@ -6,15 +6,15 @@ <%= image_tag "sul-logo-stacked.svg", class: "d-block d-sm-none", alt: "", height: 25 %> <% end %>
- <%= link_to feedback_path, class: 'first', data: { bs_toggle: 'collapse', bs_target: '#feedback-form' } do %> + <%= link_to '#feedback-form', class: 'first', data: { bs_toggle: 'collapse', bs_target: '#feedback-form' } do %> Feedback <% end if show_feedback_form? %>
<% if show_feedback_form? %> -
- - -
+
+ + +
<% end %> diff --git a/app/views/shared/feedback_forms/_form.html.erb b/app/views/shared/feedback_forms/_form.html.erb deleted file mode 100644 index a79e7bc5..00000000 --- a/app/views/shared/feedback_forms/_form.html.erb +++ /dev/null @@ -1,50 +0,0 @@ -
- <%= form_tag feedback_form_path, method: :post, class:"form-horizontal feedback-form", role:"form" do %> -
-
- <%= render "shared/feedback_forms/reporting_from" %> -
-
- - <%= label_tag(:email_address, 'Ignore this text box. It is used to detect spammers. If you enter anything into this text box, your message will not be sent.') %>
- <%= email_field_tag :email_address %>
- <%= text_field_tag :user_agent %> - <%= text_field_tag :viewport %> - -
-
- <%= label_tag(:message, 'Message', class: "col-sm-3 col-form-label text-end") %> -
- <%= text_area_tag :message, "", rows:"5", class:"form-control" %> -
-
-
- <%= label_tag(:name, 'Your name', class: "col-sm-3 col-form-label text-end") %> -
- <%= text_field_tag :name, "", class:"form-control" %> -
-
-
- <%= label_tag(:to, 'Your email', class: "col-sm-3 col-form-label text-end") %> -
- <%= email_field_tag :to, "", class:"form-control" %> -
-
- <% if current_user.blank? %> -
-
- <%= recaptcha_tags %> - -

(Stanford users can avoid this Captcha by logging in.)

-
-
- <% end %> -
-
- - <%= button_tag "Cancel", type: 'button', class: 'btn btn-link', data: { bs_toggle: 'collapse', bs_target: '#feedback-form' }, aria: { expanded: false, controls: 'feedback-form' } %> -
-
-
- <% end %> -
diff --git a/app/views/shared/feedback_forms/_reporting_from.html.erb b/app/views/shared/feedback_forms/_reporting_from.html.erb deleted file mode 100644 index bb288e77..00000000 --- a/app/views/shared/feedback_forms/_reporting_from.html.erb +++ /dev/null @@ -1,4 +0,0 @@ - diff --git a/config/importmap.rb b/config/importmap.rb index 853d894c..499c4e55 100644 --- a/config/importmap.rb +++ b/config/importmap.rb @@ -1,7 +1,6 @@ # Pin npm packages by running ./bin/importmap pin "application", preload: true -pin "jquery", to: "https://ga.jspm.io/npm:jquery@3.6.1/dist/jquery.js" pin "bootstrap", to: "https://ga.jspm.io/npm:bootstrap@5.2.3/dist/js/bootstrap.esm.js" pin "@popperjs/core", to: "https://ga.jspm.io/npm:@popperjs/core@2.11.6/lib/index.js" pin "@hotwired/stimulus", to: "stimulus.min.js", preload: true diff --git a/spec/controller/feedback_forms_controller_spec.rb b/spec/controller/feedback_forms_controller_spec.rb index 1d3a1cd2..0f643719 100644 --- a/spec/controller/feedback_forms_controller_spec.rb +++ b/spec/controller/feedback_forms_controller_spec.rb @@ -3,52 +3,45 @@ RSpec.describe FeedbackFormsController, type: :controller do before do allow(Settings.feedback).to receive(:email_to).and_return('feedback@example.com') + allow(controller).to receive(:verify_recaptcha).and_return(verify) + allow(controller).to receive(:current_user).and_return(current_user) + allow(FeedbackMailer).to receive(:submit_feedback).and_return(mailer) end - describe 'format json' do - it 'returns json success' do - post :create, params: { url: 'http://test.host/', message: 'Hello Kittenz', format: 'json' } - expect(flash[:success]).to eq 'Thank you! Your feedback has been sent.' - end - - it 'returns html success' do - post :create, params: { url: 'http://test.host/', message: 'Hello Kittenz' } - expect(flash[:success]).to eq 'Thank you! Your feedback has been sent.' - end - end + let(:verify) { false } + let(:current_user) { nil } + let(:mailer) { instance_double(ActionMailer::MessageDelivery, deliver_now: true) } describe 'validate' do - it 'returns an error if no message is sent' do - post :create, params: { url: 'http://test.host/', message: '', email_address: '' } - expect(flash[:error]).to eq FeedbackFormsController::MESSAGE_BLANK_MESSAGE - end - - it 'returns an error if a bot fills in the email_address field (email is correct field)' do - post :create, params: { message: 'I am spamming you!', url: 'http://test.host/', email_address: 'spam!' } - expect(flash[:error]).to eq FeedbackFormsController::EMAIL_PRESENT_MESSAGE + context 'when the user is not logged in and captcha fails' do + it 'returns an error if the recaptcha is incorrect' do + post :create, params: { message: 'I am spamming you!', url: 'http://test.host/' } + expect(flash[:error]).to eq 'You must pass the reCAPTCHA challenge' + expect(FeedbackMailer).not_to have_received(:submit_feedback) + end end - context 'when the user is not logged in' do - before do - allow(controller).to receive(:verify_recaptcha).and_return(false) - end + context 'when the user is not logged in and captcha succeeds' do + let(:verify) { true } - it 'returns an error if the recaptcha is incorrect' do + it 'returns success and sends email' do post :create, params: { message: 'I am spamming you!', url: 'http://test.host/' } - expect(flash[:error]).to eq FeedbackFormsController::RECAPTCHA_MESSAGE + + expect(flash[:success]).to eq 'Thank you! Your feedback has been sent.' + expect(controller).to have_received(:verify_recaptcha) + expect(FeedbackMailer).to have_received(:submit_feedback) end end context 'when the user is logged in' do - before do - allow(controller).to receive(:current_user).and_return('any truthy value, really') - allow(controller).to receive(:verify_recaptcha) - end + let(:current_user) { 'chester' } - it 'does not care if the recaptcha is incorrect' do + it 'returns success and sends email' do post :create, params: { message: 'I am spamming you!', url: 'http://test.host/' } expect(flash[:error]).to be_nil + expect(flash[:success]).to eq 'Thank you! Your feedback has been sent.' expect(controller).not_to have_received(:verify_recaptcha) + expect(FeedbackMailer).to have_received(:submit_feedback) end end end