Additional cops, mostly guard clauses

.rubocop.yml

@@ -144,4 +144,11 @@ Style/DotPosition:
 
 Style/EmptyElse:
   Enabled: true
-  EnforcedStyle: both
+  EnforcedStyle: both
+
+Style/GuardClause:
+  Enabled: true
+  MinBodyLength: 2
+
+Performance:
+  Enabled: true

app/controllers/slides_controller.rb

@@ -363,6 +363,7 @@ def full
 
   # Send a given sized slide image
   def send_slide_image(size)
+    return unless stale?(last_modified: @slide.images_updated_at.utc, etag: @slide)
     case size
     when :full
       filename = @slide.full_filename
@@ -371,22 +372,19 @@ def send_slide_image(size)
     else
       filename = @slide.preview_filename
     end
-
-    if stale?(last_modified: @slide.images_updated_at.utc, etag: @slide)
-      respond_to do |format|
-        format.html do
-          # Set content headers to allow CORS
-          response.headers["Access-Control-Allow-Origin"] = "*"
-          response.headers["Access-Control-Request-Method"] = "GET"
-          if @slide.ready
-            send_file filename, disposition: "inline"
-          else
-            send_file(Rails.root.join("data", "no_image.jpg"),
-                      disposition: "inline")
-          end
+    respond_to do |format|
+      format.html do
+        # Set content headers to allow CORS
+        response.headers["Access-Control-Allow-Origin"] = "*"
+        response.headers["Access-Control-Request-Method"] = "GET"
+        if @slide.ready
+          send_file filename, disposition: "inline"
+        else
+          send_file(Rails.root.join("data", "no_image.jpg"),
+                    disposition: "inline")
         end
-        format.js { render :show }
       end
+      format.js { render :show }
     end
   end
 
@@ -400,14 +398,12 @@ def slide_params
   end
 
   def require_create
-    unless Slide.can_create? current_user
-      fail ApplicationController::PermissionDenied
-    end
+    return if Slide.can_create? current_user
+    fail ApplicationController::PermissionDenied
   end
 
   def require_admin
-    unless Slide.admin? current_user
-      fail ApplicationController::PermissionDenied
-    end
+    return if Slide.admin? current_user
+    fail ApplicationController::PermissionDenied
   end
 end

app/controllers/tickets_controller.rb

@@ -98,8 +98,7 @@ def ticket_create_params
   end
 
   def require_admin
-    unless Ticket.admin? current_user
-      fail ApplicationController::PermissionDenied
-    end
+    return unless Ticket.admin? current_user
+    fail ApplicationController::PermissionDenied
   end
 end

app/helpers/displays_helper.rb

@@ -13,15 +13,14 @@ def late_display_warning(d)
 
   # Link to displays#destroy if user has sufficient access
   def display_destroy_button(d)
-    if d.admin?(current_user)
-      link_to icon("times-circle", "Delete"), display_path(d),
-              data: {
-                      confirm: "Are you sure you want to delete the display"\
-                               " \"#{d.name}\", this cannot be undone?"
-                    },
-              title: "Delete this display premanently",
-              method: :delete, class: "button warning"
-    end
+    return unless d.admin?(current_user)
+    link_to icon("times-circle", "Delete"), display_path(d),
+            data: {
+                    confirm: "Are you sure you want to delete the display"\
+                             " \"#{d.name}\", this cannot be undone?"
+                  },
+            title: "Delete this display premanently",
+            method: :delete, class: "button warning"
   end
 
   # Set the panel class based on display status
@@ -38,11 +37,8 @@ def display_class(display)
     else
       panel = "panel-danger"
     end
-    if display.live?
-      return "#{panel} display-live"
-    else
-      return panel
-    end
+    return "#{panel} display-live" if display.live?
+    return panel
   end
 
   # Render the display ping element
@@ -88,11 +84,8 @@ def display_current_slide(d)
 
   # Render the last_contact_at timestamp and the diff to current time
   def display_last_contact(d)
-    if d.last_contact_at
-      delta = Time.diff(Time.now, d.last_contact_at, "%h:%m:%s")[:diff]
-      return "#{l d.last_contact_at, format: :short} (#{delta} ago)"
-    else
-      return "UNKNOWN"
-    end
+    return "UNKNOWN" unless d.last_contact_at
+    delta = Time.diff(Time.now, d.last_contact_at, "%h:%m:%s")[:diff]
+    return "#{l d.last_contact_at, format: :short} (#{delta} ago)"
   end
 end

app/helpers/events_helper.rb

@@ -33,9 +33,8 @@ def event_new_button
 
   # Check if this event is current one and if it is then set the class to 'success'
   def event_current_class(event)
-    if event.current
-      "success"
-    end
+    return unless event.current
+    "success"
   end
 
   # Select box for choosing the slide resolution for the event.

app/helpers/presentations_helper.rb

@@ -13,15 +13,14 @@ def duration_to_text(dur)
 
   # Link to displays#destroy if user has sufficient access
   def presentation_destroy_button(p)
-    if p.can_edit?(current_user)
-      link_to delete_link_text, presentation_path(p),
-              data: {
-                      confirm: "Are you sure you want to delete the presentation"\
-                               " \"#{p.name}\", this cannot be undone?"
-                    },
-              title: "Delete this presentation premanently",
-              method: :delete, class: "btn btn-danger"
-    end
+    return unless p.can_edit?(current_user)
+    link_to delete_link_text, presentation_path(p),
+            data: {
+                    confirm: "Are you sure you want to delete the presentation"\
+                             " \"#{p.name}\", this cannot be undone?"
+                  },
+            title: "Delete this presentation premanently",
+            method: :delete, class: "btn btn-danger"
   end
 
   # Button to edit a presentation

app/helpers/tickets_helper.rb

@@ -7,11 +7,9 @@
 module TicketsHelper
   # Link to the object associated to this ticket
   def ticket_concerning(ticket)
-    if ticket.about.present?
-      return "#{ticket.about_type.capitalize}: #{ticket_object_link(ticket)}".html_safe
-    else
-      return "None"
-    end
+    return "None" unless ticket.about.present?
+    return "#{ticket.about_type.capitalize}:"\
+           " #{ticket_object_link(ticket)}".html_safe
   end
 
   # Render a link to the associated object on a ticket
@@ -80,28 +78,26 @@ def ticket_edit_button(ticket)
   end
 
   def ticket_close_link(ticket, html_class = nil)
-    if ticket.can_close? current_user
-      link_to icon("check-square-o", "Close"),
-              ticket_path(ticket,
-                          ticket: {
-                            status: Ticket::StatusClosed }),
-              method: :put,
-              class: html_class
-    end
+    return unless ticket.can_close? current_user
+    link_to icon("check-square-o", "Close"),
+            ticket_path(ticket,
+                        ticket: {
+                          status: Ticket::StatusClosed }),
+            method: :put,
+            class: html_class
   end
 
   def ticket_close_button(ticket)
     ticket_close_link(ticket, "btn btn-success")
   end
 
   def ticket_destroy_link(ticket, html_class = nil)
-    if ticket.admin? current_user
-      link_to icon("times-circle", "Delete"),
-              ticket_path(ticket),
-              class: html_class,
-              method: :delete,
-              data: { confirm: "Are you sure you want to permanently delete this ticket?" }
-    end
+    return unless ticket.admin? current_user
+    link_to icon("times-circle", "Delete"),
+            ticket_path(ticket),
+            class: html_class,
+            method: :delete,
+            data: { confirm: "Are you sure you want to permanently delete this ticket?" }
   end
 
   def ticket_destroy_button(ticket)
@@ -129,12 +125,9 @@ def ticket_kind(ticket)
 private
 
   def ticket_open_count
-    if Ticket.current.open.count > 0
-      html = icon "ticket"
-      html << Ticket.ticket.open.count.to_s
-      return html
-    else
-      return ""
-    end
+    return "" unless Ticket.current.open.count > 0
+    html = icon "ticket"
+    html << Ticket.ticket.open.count.to_s
+    return html
   end
 end

app/models/concerns/has_slidedata.rb

@@ -78,10 +78,9 @@ def default_slidedata
   end
 
   def write_slidedata
-    unless self.new_record?
-      File.open(self.data_filename,  "w") do |f|
-        f.write sanitalize_slidedata(@_slidedata).to_yaml
-      end
+    return if self.new_record?
+    File.open(self.data_filename,  "w") do |f|
+      f.write sanitalize_slidedata(@_slidedata).to_yaml
     end
   end
 end

app/models/concerns/has_svg_data.rb

@@ -33,13 +33,10 @@ def svg_data
   # We also mark the slide as not ready because the picture isn't current anymore and needs
   # to be regenerated.
   def svg_data=(svg)
-    if self.svg_data != svg
-
-      @_svg_data = svg
-      write_svg_data
-
-      self.ready = false
-    end
+    return unless self.svg_data != svg
+    @_svg_data = svg
+    write_svg_data
+    self.ready = false
   end
 
   # Filename to store the svg in.
@@ -53,10 +50,9 @@ def svg_filename
   # We need to check if record is new, because we don't know the filename for the svg
   # before the record is saved.
   def write_svg_data
-    unless self.new_record?
-      File.open(self.svg_filename, "wb") do |f|
-        f.write @_svg_data
-      end
+    return if self.new_record?
+    File.open(self.svg_filename, "wb") do |f|
+      f.write @_svg_data
     end
   end
 

app/models/concerns/model_authorization.rb

@@ -61,20 +61,14 @@ def can_create?(user)
 
     # List of all displays a user can add overrides on
     def can_override(user)
-      if user.has_role?([self.auth_roles[:admin], self.auth_roles[:override]])
-        return relation
-      else
-        return self.joins(:authorized_users).where(users: { id: user.id })
-      end
+      return relation if user.has_role?([self.auth_roles[:admin], self.auth_roles[:override]])
+      return self.joins(:authorized_users).where(users: { id: user.id })
     end
 
     # List of all objects user can edit
     def can_edit(user)
-      if user.has_role?(self.auth_roles[:admin])
-        return relation
-      else
-        self.joins(:authorized_users).where("users.id = ?", user.id)
-      end
+      return relation if user.has_role?(self.auth_roles[:admin])
+      self.joins(:authorized_users).where("users.id = ?", user.id)
     end
 
     # Does user have admin priviledges on all objects of this type?

app/models/concerns/websocket_messages.rb

@@ -60,10 +60,9 @@ def send_messages(event)
       display_datas
     end
 
-    if self.previous_changes.include?("images_updated_at") && event == :update
-      Rails.logger.debug "-> Slide image has been updated, sending notifications"
-      self.updated_image_notifications
-    end
+    return unless self.previous_changes.include?("images_updated_at") && event == :update
+    Rails.logger.debug "-> Slide image has been updated, sending notifications"
+    self.updated_image_notifications
   end
 
   def get_channel

app/models/display.rb

@@ -143,11 +143,8 @@ def self.late
 
   # Is this display more than Timeout minutes late?
   def late?
-    if self.last_contact_at
-      return Time.diff(Time.now, self.last_contact_at, "%m")[:diff].to_i > Timeout
-    else
-      return false
-    end
+    return false unless self.last_contact_at
+    return Time.diff(Time.now, self.last_contact_at, "%m")[:diff].to_i > Timeout
   end
 
   # Is the display live, ie. visible to the general audience
@@ -218,10 +215,9 @@ def ping
 
   # Create the associated display state as needed
   def create_state
-    if self.display_state.nil?
-      ds = DisplayState.new
-      self.display_state = ds
-    end
+    return unless self.display_state.nil?
+    ds = DisplayState.new
+    self.display_state = ds
   end
 
   # If display is in manual control also stop accepting overrides

app/models/display_state.rb

@@ -33,20 +33,19 @@ def displays
 
   # Send error notifications if we are in error state
   def send_error_notifications
-    if self.status == "error"
-      if self.display.error_tickets.open.present?
-        msg = self.display.error_tickets.open.last!.description.lines.last
-      else
-        msg = "Error has occured!"
-      end
-      data = {
-        id: self.display_id,
-        message: msg
-      }
-      Rails.logger.error "Error has occured on display #{self.display_id} with message: '#{msg}'"
-      msg = IskMessage.new("display", "error", data)
-      msg.send
-      msg.send(self.display.websocket_channel)
+    return unless self.status == "error"
+    if self.display.error_tickets.open.present?
+      msg = self.display.error_tickets.open.last!.description.lines.last
+    else
+      msg = "Error has occured!"
     end
+    data = {
+      id: self.display_id,
+      message: msg
+    }
+    Rails.logger.error "Error has occured on display #{self.display_id} with message: '#{msg}'"
+    msg = IskMessage.new("display", "error", data)
+    msg.send
+    msg.send(self.display.websocket_channel)
   end
 end