fix some rubocop issues and tweak some code

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?
+    unsubscribe_redirect_if_logged_out!
-    # need to use devise stored_url or whatever
+    check_if_already_unsubscribed!
-    if current_user.nil?
+    return if performed? # if one of these checks already redirected, we're done
-      flash[:notice] = 'Continue to unsubscribe from emails by logging in.'
-      return redirect_to "#{sign_in_path}?redirect_to=#{unsubscribe_notifications_path}"
-    end
 
-    if current_user.emails_allowed == false
+    if current_user.update(emails_allowed: false)
-      return redirect_to edit_user_path(current_user), notice: 'You were already unsubscribed from emails.'
+      redirect_to edit_user_path(current_user),
-    end
+                  notice: 'You will no longer receive emails from Metamaps.'
-
-    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.'
     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

app/controllers/users/sessions_controller.rb

@@ -1,24 +1,25 @@
-class Users::SessionsController < Devise::SessionsController
+# frozen_string_literal: true
-  after_action :store_location, only: [:new]
+module Users
+  class SessionsController < Devise::SessionsController
+    after_action :store_location, only: [:new]
 
-  protected
+    protected
 
-  def after_sign_in_path_for(resource)
+    def after_sign_in_path_for(resource)
-    stored = stored_location_for(User)
+      stored = stored_location_for(User)
-    return stored if stored
+      return stored if stored
 
-    if request.referer&.match(sign_in_url) || request.referer&.match(sign_up_url)
+      if request.referer&.match(sign_in_url) || request.referer&.match(sign_up_url)
-      super
+        super
-    else
+      else
-      request.referer || root_path
+        request.referer || root_path
+      end
     end
-  end
 
-  private
+    private
 
-  def store_location
+    def store_location
-    if params[:redirect_to]
+      store_location_for(User, params[:redirect_to]) if params[:redirect_to]
-      store_location_for(User, params[:redirect_to])    
     end
   end
 end

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

app/models/user.rb

@@ -114,11 +114,8 @@ class User < ApplicationRecord
   # Mailboxer hooks and helper functions
 
   def mailboxer_email(_message)
-    if emails_allowed
+    return email if emails_allowed
-      email
+    # else return nil, which sends no email
-    else
-      nil
-    end
   end
 
   def mailboxer_notifications

app/views/notifications/index.html.erb

@@ -16,9 +16,9 @@
           </div>
           <div class="notification-read-unread">
             <% 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 %>
           </div>
         </li>

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')

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')

app/views/shared/_back_to_mapping.html.erb

@@ -1,3 +1,3 @@
 <div class="centerContent back-to-mapping">
-  <a href="/">Back to mapping!</a>
+  <a href="/">Back to mapping</a>
 </div>

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 = if ENV['ACTIVE_JOB_FRAMEWORK'] == 'sucker_punch'
-      config.active_job.queue_adapter = :sucker_punch
+                                        :sucker_punch
-    else
+                                      else
-      config.active_job.queue_adapter = :delayed_job
+                                        :delayed_job
-    end
+                                      end
 
     # Custom directories with classes and modules you want to be autoloadable.
     config.autoload_paths << Rails.root.join('app', 'services')