From 689a965f9d876d489a908f0172ebc8843c443264 Mon Sep 17 00:00:00 2001 From: Devin Howard Date: Mon, 28 Nov 2016 12:54:33 -0500 Subject: [PATCH] fix some rubocop issues and tweak some code --- app/controllers/notifications_controller.rb | 34 +++++++++++--------- app/controllers/users/sessions_controller.rb | 31 +++++++++--------- app/mailers/application_mailer.rb | 2 +- app/models/user.rb | 7 ++-- app/views/notifications/index.html.erb | 4 +-- app/views/notifications/mark_read.js.erb | 2 +- app/views/notifications/mark_unread.js.erb | 2 +- app/views/shared/_back_to_mapping.html.erb | 2 +- config/application.rb | 10 +++--- 9 files changed, 48 insertions(+), 46 deletions(-) diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 12a1116c..4759ef20 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -55,22 +55,13 @@ class NotificationsController < ApplicationController end def unsubscribe - # TODO will a logged out user be unsubscribed after logging in? - # need to use devise stored_url or whatever - if current_user.nil? - flash[:notice] = 'Continue to unsubscribe from emails by logging in.' - return redirect_to "#{sign_in_path}?redirect_to=#{unsubscribe_notifications_path}" - end + unsubscribe_redirect_if_logged_out! + check_if_already_unsubscribed! + return if performed? # if one of these checks already redirected, we're done - if current_user.emails_allowed == false - return redirect_to edit_user_path(current_user), notice: 'You were already unsubscribed from emails.' - end - - current_user.emails_allowed = false - success = current_user.save - - if success - redirect_to edit_user_path(current_user), notice: 'You will no longer receive emails from Metamaps.' + if current_user.update(emails_allowed: false) + redirect_to edit_user_path(current_user), + notice: 'You will no longer receive emails from Metamaps.' else flash[:alert] = 'Sorry, something went wrong. You have not been unsubscribed from emails.' redirect_to edit_user_path(current_user) @@ -79,6 +70,19 @@ class NotificationsController < ApplicationController private + def unsubscribe_redirect_if_logged_out! + return if current_user.present? + + flash[:notice] = 'Continue to unsubscribe from emails by logging in.' + redirect_to "#{sign_in_path}?redirect_to=#{unsubscribe_notifications_path}" + end + + def check_if_already_unsubscribed! + return if current_user.emails_allowed + + redirect_to edit_user_path(current_user), notice: 'You were already unsubscribed from emails.' + end + def set_receipts @receipts = current_user.mailboxer_notification_receipts end diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 73d6ea73..1c9c0a1e 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -1,24 +1,25 @@ -class Users::SessionsController < Devise::SessionsController - after_action :store_location, only: [:new] +# frozen_string_literal: true +module Users + class SessionsController < Devise::SessionsController + after_action :store_location, only: [:new] - protected + protected - def after_sign_in_path_for(resource) - stored = stored_location_for(User) - return stored if stored + def after_sign_in_path_for(resource) + stored = stored_location_for(User) + return stored if stored - if request.referer&.match(sign_in_url) || request.referer&.match(sign_up_url) - super - else - request.referer || root_path + if request.referer&.match(sign_in_url) || request.referer&.match(sign_up_url) + super + else + request.referer || root_path + end end - end - private + private - def store_location - if params[:redirect_to] - store_location_for(User, params[:redirect_to]) + def store_location + store_location_for(User, params[:redirect_to]) if params[:redirect_to] end end end diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index c73b28f2..10961836 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -4,6 +4,6 @@ class ApplicationMailer < ActionMailer::Base layout 'mailer' def deliver - fail NotImplementedError('Please use Mailboxer to send your emails.') + raise NotImplementedError('Please use Mailboxer to send your emails.') end end diff --git a/app/models/user.rb b/app/models/user.rb index ef11e5f6..f6fcb60e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -114,11 +114,8 @@ class User < ApplicationRecord # Mailboxer hooks and helper functions def mailboxer_email(_message) - if emails_allowed - email - else - nil - end + return email if emails_allowed + # else return nil, which sends no email end def mailboxer_notifications diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb index 3b178fcc..2664fc8d 100644 --- a/app/views/notifications/index.html.erb +++ b/app/views/notifications/index.html.erb @@ -16,9 +16,9 @@
<% if receipt.is_read? %> - <%= link_to '(read)', mark_unread_notification_path(notification.id), remote: true, method: :put %> + <%= link_to 'mark as unread', mark_unread_notification_path(notification.id), remote: true, method: :put %> <% else %> - <%= link_to '(unread)', mark_read_notification_path(notification.id), remote: true, method: :put %> + <%= link_to 'mark as read', mark_read_notification_path(notification.id), remote: true, method: :put %> <% end %>
diff --git a/app/views/notifications/mark_read.js.erb b/app/views/notifications/mark_read.js.erb index 631f3f0d..5b28f453 100644 --- a/app/views/notifications/mark_read.js.erb +++ b/app/views/notifications/mark_read.js.erb @@ -1,5 +1,5 @@ $('#notification-<%= @notification.id %> .notification-read-unread > a') - .text('(read)') + .text('mark as unread') .attr('href', '<%= mark_unread_notification_path(@notification.id) %>') $('#notification-<%= @notification.id %>') .removeClass('unread') diff --git a/app/views/notifications/mark_unread.js.erb b/app/views/notifications/mark_unread.js.erb index cc17c4d1..46744388 100644 --- a/app/views/notifications/mark_unread.js.erb +++ b/app/views/notifications/mark_unread.js.erb @@ -1,5 +1,5 @@ $('#notification-<%= @notification.id %> .notification-read-unread > a') - .text('(unread)') + .text('mark as read') .attr('href', '<%= mark_read_notification_path(@notification.id) %>') $('#notification-<%= @notification.id %>') .removeClass('read') diff --git a/app/views/shared/_back_to_mapping.html.erb b/app/views/shared/_back_to_mapping.html.erb index 1850811a..682a71e6 100644 --- a/app/views/shared/_back_to_mapping.html.erb +++ b/app/views/shared/_back_to_mapping.html.erb @@ -1,3 +1,3 @@
- Back to mapping! + Back to mapping
diff --git a/config/application.rb b/config/application.rb index c60e0bf0..ff5a621c 100644 --- a/config/application.rb +++ b/config/application.rb @@ -12,11 +12,11 @@ module Metamaps # Application configuration should go into files in config/initializers # -- all .rb files in that directory are automatically loaded. # - if ENV['ACTIVE_JOB_FRAMEWORK'] == 'sucker_punch' - config.active_job.queue_adapter = :sucker_punch - else - config.active_job.queue_adapter = :delayed_job - end + config.active_job.queue_adapter = if ENV['ACTIVE_JOB_FRAMEWORK'] == 'sucker_punch' + :sucker_punch + else + :delayed_job + end # Custom directories with classes and modules you want to be autoloadable. config.autoload_paths << Rails.root.join('app', 'services')