Rename/refactor in models for clarity

app/controllers/events_controller.rb

@@ -315,7 +315,7 @@ def index
     end
 
     @eventdates = @eventdates.order("startdate ASC").includes({event: [:organization]}, {event_roles: [:member]}, :locations, :equipment_profile).references(:event)
-    @eventweeks = Eventdate.weekify(@eventdates)
+    @eventweeks = Eventdate.group_by_weeks_until(@eventdates)
 
     if cannot? :read, Event  # kiosk, non-signed-in
       render(:action => "index", :layout => "public")
@@ -338,7 +338,7 @@ def month
       @eventdates = Eventdate.where("enddate >= ? AND startdate <= ? AND events.publish = true", @startdate.utc, enddate.utc).order("startdate ASC").includes(:event).references(:event)
     end
 
-    @eventruns = Eventdate.runify(@eventdates)
+    @eventruns = Eventdate.group_by_runcrew(@eventdates)
 
     if cannot? :read, Event  # kiosk, non-signed-in
       render(:action => "month", :layout => "public")
@@ -350,23 +350,23 @@ def incomplete
     authorize! :read, Event
 
     @eventdates = Eventdate.where("NOT events.status IN (?)", Event::Event_Status_Group_Completed).order("startdate ASC").includes(:event).references(:event)
-    @eventruns = Eventdate.runify(@eventdates)
+    @eventruns = Eventdate.group_by_runcrew(@eventdates)
   end
 
   def past
     @title = "Past Event List"
     authorize! :read, Event
 
     @eventdates = Eventdate.where("startdate <= ?", Time.now.utc).order("startdate DESC").paginate(:per_page => 50, :page => params[:page])
-    @eventruns = Eventdate.runify(@eventdates)
+    @eventruns = Eventdate.group_by_runcrew(@eventdates)
   end
 
   def search
     @title = "Event List - Search for " + params[:q]
     authorize! :read, Event
 
     @eventdates = Eventdate.search params[:q].gsub(/[^A-Za-z0-9 ]/,""), :page => params[:page], :per_page => 50, :order => "startdate DESC"
-    @eventruns = Eventdate.runify(@eventdates)
+    @eventruns = Eventdate.group_by_runcrew(@eventdates)
   end
 
   def calendar

app/controllers/members_controller.rb

@@ -70,7 +70,7 @@ def create
     @member.password_confirmation = password
 
     if @member.save
-      if not @member.andrew?
+      if not @member.andrew_email?
         raw_token, hashed_token = Devise.token_generator.generate(Member, :reset_password_token)
         @member.reset_password_token = hashed_token
         @member.reset_password_sent_at = Time.now.utc

app/helpers/events_helper.rb

@@ -150,9 +150,9 @@ def show_run_position(er, hover)
       return link_to("you?", new_application_url(er.event, event_role_id: er.id, format: :js), :remote => true) unless hover
       "you?" if hover
     elsif !hover and current_member
-      er.assigned_to use_display_name: true
+      er.assigned_to_name use_display_name: true
     else
-      er.assigned_to
+      er.assigned_to_name
     end
   end
 

app/mailers/event_role_application_mailer.rb

@@ -4,7 +4,7 @@ def apply(application, notes)
     @application = application
     @notes = notes
 
-    mail to: application.superior_email, from: "no-reply@abtech.andrew.cmu.edu", subject: "Application for #{application.event_role.description} #{application.event_role.role} from #{application.member.display_name}"
+    mail to: application.superior_emails, from: "no-reply@abtech.andrew.cmu.edu", subject: "Application for #{application.event_role.description} #{application.event_role.role} from #{application.member.display_name}"
   end
 
   def accept(application)
@@ -16,6 +16,6 @@ def accept(application)
   def withdraw(application)
     @application = application
 
-    mail to: application.superior_email, from: "no-reply@abtech.andrew.cmu.edu", subject: "Application withdrawn for #{application.event_role.description} #{application.event_role.role} from #{application.member.display_name}"
+    mail to: application.superior_emails, from: "no-reply@abtech.andrew.cmu.edu", subject: "Application withdrawn for #{application.event_role.description} #{application.event_role.role} from #{application.member.display_name}"
   end
 end

app/models/account.rb → app/models/current_academic_year.rb

@@ -1,14 +1,14 @@
-class Account
+class CurrentAcademicYear
 
-  def self.magic_date
+  def self.start_date
     if Date.today.month < 7
       (Date.today.year - 1).to_s + '-07-01'
     else
       Date.today.year.to_s + '-07-01'
     end
   end
 
-  def self.future_magic_date
+  def self.end_date
     if Date.today.month < 7
       Date.today.year.to_s + '-07-01'
     else

app/models/equipment_profile.rb

@@ -21,6 +21,6 @@ def full_category
 
   private
     def null_subcategory
-      subcategory = nil if subcategory.blank?
+      self.subcategory = nil if subcategory.blank?
     end
 end

app/models/event.rb

@@ -21,7 +21,7 @@ class Event < ActiveRecord::Base
   attr_accessor :org_type, :org_new, :created_email
 
   before_validation :prune_attachments, :prune_roles
-  before_save :handle_organization, :ensure_tic, :sort_roles, :synchronize_representative_dates
+  before_save :create_org_if_new, :ensure_tic, :sort_roles, :synchronize_representative_dates
   after_initialize :default_values
   after_save :set_eventdate_delta_flags, :set_created_email
 
@@ -71,7 +71,7 @@ class Event < ActiveRecord::Base
   # validate :eventdate_valid?
   validate :textable_social_valid?
 
-  scope :current_year, -> { where("representative_date >= ? or last_representative_date > ?", Account.magic_date, Account.magic_date) }
+  scope :current_year, -> { where("representative_date >= ? or last_representative_date > ?", CurrentAcademicYear.start_date, CurrentAcademicYear.start_date) }
 
   ThinkingSphinx::Callbacks.append(self, :behaviours => [:sql, :deltas])  # associated via eventdate
 
@@ -88,9 +88,12 @@ def to_s
   end
 
   def members
-    @members or @members = event_roles.inject(Array.new) do |uniq_roles, er| 
-      ( uniq_roles << er.member unless er.member.nil? or uniq_roles.any? { |ur| ur.id == er.member_id } ) or uniq_roles 
+    if @members
+      return @members
     end
+
+    non_unique_members = event_roles.map(&:member).select{ |m| not m.nil?}
+    @members = non_unique_members.uniq(&:m.id)
   end
 
   def total_payroll
@@ -118,7 +121,7 @@ def tic_and_stic_only
   end
 
   def synchronize_representative_dates
-    self.representative_date = self.eventdates[0].startdate
+    self.representative_date = self.eventdates.first.startdate
     self.last_representative_date = eventdates.last.enddate
   end
 
@@ -158,11 +161,11 @@ def current_year?
     # a part of both years (i.e. Precollege). Otherwise Tracker does not allow
     # certain functions (like invoicing) for the now previous year's event. So,
     # we considered an event by start and end.
-    (representative_date >= Account.magic_date) or (self.last_representative_date > Account.magic_date)
+    (representative_date >= CurrentAcademicYear.start_date) or (self.last_representative_date > CurrentAcademicYear.start_date)
   end
 
   private
-    def handle_organization
+    def create_org_if_new
       if self.org_type == "new"
         self.organization = Organization.create(:name => org_new)
       end
@@ -220,7 +223,7 @@ def set_created_email
     end
 
     def eventdate_valid?
-      if representative_date < Account.magic_date
+      if representative_date < CurrentAcademicYear.start_date
        errors.add(:representative_date, "Requested date out of range")
       end
     end

app/models/event_role.rb

@@ -107,10 +107,10 @@ def assigned?
   end
 
   def to_s
-    role + ": " + assigned_to
+    role + ": " + assigned_to_name
   end
 
-  def assigned_to(options = {})
+  def assigned_to_name(options = {})
     if assigned?
       if options[:use_display_name]
         member.display_name
@@ -122,16 +122,14 @@ def assigned_to(options = {})
     end
   end
 
-  def sort_index 
-    Roles_All.each_index { |role_index| return role_index if Roles_All[role_index] == role }
-    return -1 
+  def sort_index
+    index = Roles_All.index self.role
+    index.nil? ? -1 : index  # Replace nil with -1 if not found
   end
 
   # define the natural sorting order
-  def <=> (role)
-    return 1 if sort_index < 0
-    return -1 if role.sort_index < 0
-    return sort_index <=> role.sort_index   
+  def <=> (other_role)
+    sort_index <=> other_role.sort_index
   end
 
   def assistants

app/models/event_role_application.rb

@@ -5,27 +5,27 @@ class EventRoleApplication < ApplicationRecord
   validates_presence_of :event_role, :member
   validate :event_role_is_appliable
 
-  def superior
+  def superiors
     position = event_role.superior
     unless position.nil?
       sup = event_role.roleable.event_roles.where(role: position).where.not(member: nil)
       return sup.map(&:member) unless sup.empty?
     end
 
-    sup = event_role.roleable.tic_and_stic_only
-    return sup unless sup.empty?
+    # If there are no members with a superior role
+    # TiC is next in line
 
-    []
+    event_role.roleable.tic_and_stic_only
   end
 
-  def superior_name
-    sup = superior
+  def superior_names
+    sup = superiors
     return sup.map(&:display_name) unless sup.empty?
     ["the Head of Tech"]
   end
 
-  def superior_email
-    sup = superior
+  def superior_emails
+    sup = superiors
     return sup.map(&:email) unless sup.empty?
     ["abtech@andrew.cmu.edu"]
   end

app/models/eventdate.rb

@@ -13,7 +13,7 @@ class Eventdate < ApplicationRecord
 
   validates_presence_of :startdate, :enddate, :description, :locations, :calltype, :striketype
   validates_associated :locations, :equipment_profile
-  validate :dates, :validate_call, :validate_strike
+  validate :validate_chronologicity, :validate_call, :validate_strike
 
   before_validation :prune_roles
   after_save :synchronize_representative_dates
@@ -37,7 +37,7 @@ class Eventdate < ApplicationRecord
 
   ThinkingSphinx::Callbacks.append(self, :behaviours => [:sql, :deltas])
 
-  def dates
+  def validate_chronologicity
     if startdate and enddate
       errors.add(:base, "We're not a time machine. (End Date can't be before Start Date)") unless startdate < enddate
     end
@@ -52,22 +52,38 @@ def validate_strike
   end
 
   def valid_call?
-    (calltype != "literal") || (
-          (calldate.to_i <= startdate.to_i) &&
-          ((startdate.to_i - calldate.to_i) < Event_Span_Seconds))
+    if calltype == "literal"
+      call_before_start = calldate.to_i <= startdate.to_i
+      call_not_too_early = (startdate.to_i - calldate.to_i) < Event_Span_Seconds
+
+      return call_before_start && call_not_too_early
+    end
+
+    # Call is blank or start
+    true
   end
 
   def valid_strike?
-    (striketype != "literal") || (
-          (strikedate.to_i >= enddate.to_i) &&
-          ((strikedate.to_i - enddate.to_i) < Event_Span_Seconds))
+    if striketype == "literal"
+      strike_after_end = strikedate.to_i >= enddate.to_i
+      strike_not_too_late = (strikedate.to_i - enddate.to_i) < Event_Span_Seconds
+
+      return strike_after_end && strike_not_too_late
+    end
+
+    # Strike is blank, start, or none
+    true
   end
 
   def has_call?
+    # Yes: literal, startdate
+    # No : blank
     self.calltype == "literal" or self.calltype == "startdate"
   end
 
   def has_strike?
+    # Yes: literal, enddate
+    # No : blank, none
     self.striketype == "literal" or self.striketype == "enddate"
   end
 
@@ -120,6 +136,9 @@ def full_roles
     else
       roles = self.event_roles
 
+      # If eventdate doesn't have these roles,
+      # copy them from the event
+
       if not roles.any? { |r| r.role == EventRole::Role_HoT }
         roles += self.event.event_roles.find_all { |r| r.role == EventRole::Role_HoT }
       end
@@ -172,25 +191,27 @@ def run_positions_for(member)
     self.event_roles.where(member: member)
   end
 
-  def self.runify(eventdates)
-    eventdates.chunk do |ed|
-      ed.full_roles
-    end.map do |roles, run|
-      run
-    end
+  def self.group_by_runcrew(eventdates)
+    # In the event list, we want to be able
+    # to share one runcrew list with multiple
+    # adjacent eventdate rows if they are identical
+    eventdates.chunk(&:full_roles).map { |roles, eds| eds }
   end
 
-  def self.weekify(eventdates)
+  def self.group_by_weeks_until(eventdates)
+    # This drives the main events list
+    # being grouped by weeks until
+
     eventdates.chunk do |ed|
       if ed.startdate < DateTime.now.beginning_of_week
         0
       else
         DateTime.now.beginning_of_week.upto(ed.startdate.to_datetime).count.fdiv(7).floor  # https://stackoverflow.com/a/35092981
       end
-    end.map do |weeks, eds|
+    end.map do |weeks_away, eds|
       {
-        :weeks_away => weeks,
-        :eventruns => Eventdate.runify(eds)
+        :weeks_away => weeks_away,
+        :eventruns => Eventdate.group_by_runcrew(eds)
       }
     end
   end

app/models/invoice_line.rb

@@ -16,9 +16,12 @@ def total
     price * quantity
   end
 
-  def <=> (il)
-    return 1 if Invoice_Categories.find_index(category).nil?
-    return -1 if Invoice_Categories.find_index(il.category).nil?
-    return Invoice_Categories.find_index(category) <=> Invoice_Categories.find_index(il.category)
+  def sort_key
+    index = Invoice_Categories.find_index(category)
+    index.nil? ? -1 : index  # Replace nil with -1 if not found
+  end
+
+  def <=> (other)
+    sort_key <=> other.sort_key
   end
 end

app/models/member.rb

@@ -79,7 +79,7 @@ def ability
     @ability ||= Ability.new(self)
   end
 
-  def andrew?
+  def andrew_email?
     email.end_with? "@andrew.cmu.edu"
   end
 end

app/models/timecard.rb

@@ -16,7 +16,7 @@ class Timecard < ApplicationRecord
 
   def self.valid_eventdates
     timecards = self.valid_timecards
-    return Eventdate.where(["startdate >= ? AND events.billable = ?", Account.magic_date, true]).includes(:event).references(:event).sort_by{|ed| ed.event.title} if timecards.size == 0
+    return Eventdate.where(["startdate >= ? AND events.billable = ?", CurrentAcademicYear.start_date, true]).includes(:event).references(:event).sort_by{|ed| ed.event.title} if timecards.size == 0
     start_date, end_date = timecards.inject([nil,nil]) do |pair, timecard|
       [
         ((pair[0].nil? or timecard.start_date < pair[0]) ? timecard.start_date : pair[0]),