From 3c356cbaaf36252214135bb35bb3f20fed2ececf Mon Sep 17 00:00:00 2001
From: Aditya Prakash <aditya.prakash132@gmail.com>
Date: Sun, 17 Apr 2016 10:48:56 +0530
Subject: [PATCH 1/3] Remove unused functions from user model

set_up is callback and should be a private method.
removed methods:
* ticket(id) was introduced in 5f8a8ac6, however never used.
* self.prepare(params) only use instance was removed in d21e19b9
* attending_conference?(conference) introduced in merge of person and user model 23bf55b6,
  any previous use instance difficult to find
* biography_word_count: we are using js for this now
* biography_limit only use instance was removed in 23bf55b6
---
 app/models/user.rb | 47 ++++------------------------------------------
 1 file changed, 4 insertions(+), 43 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index d548853e..3b282955 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -61,13 +61,6 @@ class User < ActiveRecord::Base
     self.subscriptions.find_by(conference_id: conference.id).present?
   end
 
-  # Returns the purchased ticket
-  # ====Returns
-  # * +TicketUser::ActiveRecord_Relation+ -> user
-  def ticket(id)
-    ticket_purchases.where(ticket_id: id).first
-  end
-
   def supports? conference
     ticket_purchases.find_by(conference_id: conference.id).present?
   end
@@ -122,12 +115,6 @@ class User < ActiveRecord::Base
     user
   end
 
-  def setup_role
-    if User.count == 1 && User.first.email == 'deleted@localhost.osem'
-      self.is_admin = true
-    end
-  end
-
   # Gets the roles of the user, groups them by role.name and returns the resource(s) of each role
   # ====Returns
   # * +Hash+ * ->  e.g. 'organizer' =>  "(conf1, conf2)"
@@ -140,20 +127,6 @@ class User < ActiveRecord::Base
     result
   end
 
-  def self.prepare(params)
-    email = params['email']
-    user = User.where(email: email).first_or_initialize
-
-    # If there is a new user, add the necessary attributes
-    if user.new_record?
-      user.password = Devise.friendly_token[0, 20]
-      user.skip_confirmation!
-      user.attributes = params
-    end
-
-    user
-  end
-
   def registered
     registrations = self.registrations
     if registrations.count == 0
@@ -176,11 +149,6 @@ class User < ActiveRecord::Base
     !confirmed_at.nil?
   end
 
-  def attending_conference?(conference)
-    Registration.where(conference_id: conference.id,
-                       user_id: id).count
-  end
-
   def proposals(conference)
     events.where('program_id = ? AND event_users.event_role=?', conference.program.id, 'submitter')
   end
@@ -189,18 +157,11 @@ class User < ActiveRecord::Base
     proposals(conference).count
   end
 
-  def biography_word_count
-    if biography.nil?
-      0
-    else
-      biography.split.size
-    end
-  end
-
   private
 
-  def biography_limit
-    errors.add(:abstract, 'cannot have more than 150 words') if biography &&
-                                                                biography.split.size > 150
+  def setup_role
+    if User.count == 1 && User.first.email == 'deleted@localhost.osem'
+      self.is_admin = true
+    end
   end
 end

From 7e04a267cb03f4ded76935c9ca9b95b91c325b90 Mon Sep 17 00:00:00 2001
From: Aditya Prakash <aditya.prakash132@gmail.com>
Date: Sun, 17 Apr 2016 11:32:06 +0530
Subject: [PATCH 2/3] Fix get_roles method of User model

Previous implementation was associating resources with all
the roles regardless of user role in the resource. ie:
if user is cfp member of conf1, the method was returning:
{ organizer => conf1,
  cfp => conf1,
  Info Desk => conf1,
  Volunteers Coordinator => conf1
}
---
 app/helpers/application_helper.rb       |  2 +-
 app/models/user.rb                      | 12 ++++++++----
 spec/fixtures/test.txt                  |  1 -
 spec/helpers/application_helper_spec.rb | 10 ++++++++++
 4 files changed, 19 insertions(+), 6 deletions(-)
 delete mode 100644 spec/fixtures/test.txt
 create mode 100644 spec/helpers/application_helper_spec.rb

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 3fd34c53..149d5874 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -274,7 +274,7 @@ module ApplicationHelper
   # Outputs the roles of a user, including the conferences for which the user has the roles
   # Eg. organizer(oSC13, oSC14), cfp(oSC12, oSC13)
   def show_roles(roles)
-    roles.map { |x| x[0].titleize + ' ' + x[1] }.join ', '
+    roles.map{ |x| x[0].titleize + ' (' + x[1].join(', ') + ')' }.join ', '
   end
 
   def can_manage_volunteers(conference)
diff --git a/app/models/user.rb b/app/models/user.rb
index 3b282955..99eb8e98 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -117,12 +117,16 @@ class User < ActiveRecord::Base
 
   # Gets the roles of the user, groups them by role.name and returns the resource(s) of each role
   # ====Returns
-  # * +Hash+ * ->  e.g. 'organizer' =>  "(conf1, conf2)"
+  # * +Hash+ * ->  e.g. 'organizer' =>  [conf1, conf2]
   def get_roles
     result = {}
-    Role.all.find_each do |role|
-      resources = self.roles.map{ |myrole| Conference.find(myrole.resource_id).short_title }.join ', '
-      result[role.name] = "(#{ resources })" unless resources.blank?
+    roles.each do |role|
+      resource = Conference.find(role.resource_id).short_title
+      if result[role.name].nil?
+        result[role.name] = [resource]
+      else
+        result[role.name] << resource
+      end
     end
     result
   end
diff --git a/spec/fixtures/test.txt b/spec/fixtures/test.txt
deleted file mode 100644
index 7dc5f421..00000000
--- a/spec/fixtures/test.txt
+++ /dev/null
@@ -1 +0,0 @@
-Lorem ipsum dolorem...
\ No newline at end of file
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
new file mode 100644
index 00000000..fd302d85
--- /dev/null
+++ b/spec/helpers/application_helper_spec.rb
@@ -0,0 +1,10 @@
+require 'spec_helper'
+
+describe ApplicationHelper, type: :helper do
+  describe 'show_roles' do
+    it 'formats the hash passed' do
+      roles = { 'organizer' => ['oSC16', 'oSC15'], 'cfp' => ['oSC16'] }
+      expect(show_roles(roles)).to eq 'Organizer (oSC16, oSC15), Cfp (oSC16)'
+    end
+  end
+end

From 3bb220d3a9af42a32d24d00a8645171593314cba Mon Sep 17 00:00:00 2001
From: Aditya Prakash <aditya.prakash132@gmail.com>
Date: Sun, 17 Apr 2016 11:58:09 +0530
Subject: [PATCH 3/3] Add test for user model

bang(!) was removed cause it was not needed and slowing single
test runs
---
 spec/models/user_spec.rb | 339 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 299 insertions(+), 40 deletions(-)

diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index bbfd5f1d..7306987d 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -2,61 +2,320 @@ require 'spec_helper'
 
 describe User do
 
-  # It is necessary to use bang version of let to build roles before user
-  let!(:user_admin) { create(:admin) }
-  let!(:conference) { create(:conference) }
-  let!(:organizer_role) { Role.find_by(name: 'organizer', resource: conference) }
-  let!(:cfp_role) { Role.find_by(name: 'cfp', resource: conference) }
-  let!(:volunteers_coordinator_role) { Role.find_by(name: 'volunteers_coordinator', resource: conference) }
-  let!(:organizer) { create(:user, role_ids: [organizer_role.id]) }
-  let!(:user) { create(:user) }
+  let(:user_admin) { create(:admin) }
+  let(:conference) { create(:conference, short_title: 'oSC16', title: 'openSUSE Conference 2016') }
+  let(:conference2) { create(:conference, short_title: 'oSC15', title: 'openSUSE Conference 2015') }
+  let(:organizer_role) { Role.find_by(name: 'organizer', resource: conference) }
+  let(:cfp_role) { Role.find_by(name: 'cfp', resource: conference) }
+  let(:volunteers_coordinator_role) { Role.find_by(name: 'volunteers_coordinator', resource: conference) }
+  let(:organizer) { create(:user, role_ids: [organizer_role.id]) }
+  let(:user) { create(:user) }
 
-  it 'User.for_ichain_username raises exception if user is disabled' do
-    user.is_disabled = true
-    user.save
-    expect{User.for_ichain_username(user.username, email: user.email)}.to raise_error(UserDisabled)
+  describe 'validation' do
+    it 'has a valid factory' do
+      expect(build(:user)).to be_valid
+    end
+
+    it { is_expected.to validate_presence_of(:email) }
+    it { is_expected.to validate_presence_of(:username) }
+    it { is_expected.to validate_uniqueness_of(:username) }
   end
 
-  it 'returns the correct role' do
-    expect(user_admin.is_admin).to eq(true)
-    expect(organizer.roles.first).to eq(organizer_role)
+  describe 'association' do
+    it { is_expected.to have_many(:openids) }
+    it { is_expected.to have_many(:event_users).dependent(:destroy) }
+    it { is_expected.to have_many(:events).through(:event_users) }
+    it { is_expected.to have_many(:registrations).dependent(:destroy) }
+    it { is_expected.to have_many(:ticket_purchases).dependent(:destroy) }
+    it { is_expected.to have_many(:tickets).through(:ticket_purchases) }
+    it { is_expected.to have_many(:votes).dependent(:destroy) }
+    it { is_expected.to have_many(:subscriptions).dependent(:destroy) }
   end
 
-  it 'returns the correct roles' do
-    roles = [organizer_role.id, cfp_role.id]
-    another_user = create(:user, email: 'participant@example.de')
-    another_user.role_ids = roles
-    another_user.save
+  describe 'scope and nested attribute' do
+    it { should accept_nested_attributes_for :roles }
 
-    expect(another_user.roles.length).to eq(2)
-    expect(another_user.roles[0]).to eq(organizer_role)
-    expect(another_user.roles[1]).to eq(cfp_role)
-  end
+    describe '.admin' do
+      it 'includes users with admin flag' do
+        expect(User.admin).to include(user_admin)
+      end
 
-  describe '#name' do
-    it 'returns the username as name if there is not name' do
-      user = create(:user, name: nil)
-      expect(user.name).to eq(user.username)
+      it 'excludes users without admin flag' do
+        expect(User.admin).not_to include(user)
+      end
+    end
+
+    describe '.comment_notifiable' do
+      let(:cfp_user) { create(:user, role_ids: [cfp_role.id]) }
+
+      it 'includes organizer and cfp user' do
+        expect(User.comment_notifiable(conference)).to include(organizer, cfp_user)
+      end
+
+      it 'excludes ordinary user' do
+        expect(User.comment_notifiable(conference)).not_to include(user)
+      end
     end
   end
 
-  describe '#has_role?' do
-    describe 'when user has a role' do
-      it 'returns true when the user has the role' do
-        user = create(:user, role_ids: organizer_role.id)
-        expect(user.has_role?('organizer', conference)).to be true
+  describe 'methods' do
+    describe '#name' do
+      it 'returns the username as name if there is not name' do
+        user = create(:user, name: nil)
+        expect(user.name).to eq(user.username)
+      end
+    end
+
+    describe '#subscribed?' do
+      context 'user has subscribed to conference' do
+        before { create(:subscription, user: user, conference: conference) }
+
+        it 'returns true' do
+          expect(user.subscribed?(conference)).to be true
+        end
       end
 
-      it 'returns false when the user does not have the role' do
-        user = create(:user, role_ids: cfp_role.id)
+      context 'user has not subscribed to conference' do
+        it 'return false' do
+          expect(user.subscribed?(conference)).to be false
+        end
+      end
+    end
+
+    describe '.supports?' do
+      context 'user has bought tickets' do
+        before { create(:ticket_purchase, user: user, conference: conference) }
+
+        it 'returns true' do
+          expect(user.supports?(conference)).to be true
+        end
+      end
+
+      context 'user has not bought any ticket' do
+        it 'return false' do
+          expect(user.supports?(conference)).to be false
+        end
+      end
+    end
+
+    describe '.for_ichain_username' do
+      before { user.update_attributes(current_sign_in_at: Date.new(2014, 12, 12)) }
+
+      context 'user exists' do
+        it 'updates last_sign_in_at of user' do
+          expect do
+            User.for_ichain_username(user.username, email: user.email)
+            user.reload
+          end.to change { user.last_sign_in_at }
+        end
+
+        it 'updates current_sign_in_at of user' do
+          expect do
+            User.for_ichain_username(user.username, email: user.email)
+            user.reload
+          end.to change { user.current_sign_in_at }
+        end
+      end
+
+      context 'user is disabled' do
+        before { user.update_attributes(is_disabled: true) }
+
+        it 'User.for_ichain_username raises exception if user is disabled' do
+          expect{ User.for_ichain_username(user.username, email: user.email) }
+            .to raise_error(UserDisabled)
+        end
+      end
+    end
+
+    describe '.find_for_database_authentication' do
+      context 'login with username' do
+        it 'can find user by jumbled username' do
+          scrambled_username = user.username.chars.map{|c| rand > 0.5 ? c.capitalize : c}.join
+          expect(User.find_for_database_authentication(login: scrambled_username)).to eq(user)
+        end
+      end
+
+      context 'login with email' do
+        it 'can find user by jumbled email' do
+          scrambled_email = user.email.chars.map{|c| rand > 0.5 ? c.capitalize : c}.join
+          expect(User.find_for_database_authentication(login: scrambled_email)).to eq(user)
+        end
+      end
+    end
+
+    describe '.find_for_auth' do
+      let(:auth) do
+        OmniAuth::AuthHash.new(provider: 'google',
+                               uid: 'google-test-uid-1',
+                               info: {
+                                 name: 'new user name',
+                                 email: 'test-1@gmail.com',
+                                 username: 'newuser'
+                               },
+                               credentials: {
+                                 token: 'mock_token',
+                                 secret: 'mock_secret'
+                               }
+                              )
+      end
+
+      context 'user is not signed in' do
+        context 'first visit to website' do
+          before { @auth_user = User.find_for_auth(auth, nil) }
+
+          it 'initializes new user' do
+            expect(@auth_user.new_record?).to be true
+          end
+
+          it 'sets name, email, username and password' do
+            regex_base64 = %r{^(?:[A-Za-z_\-0-9+\/]{4}\n?)*(?:[A-Za-z_\-0-9+\/]{2}|[A-Za-z_\-0-9+\/]{3}=)?$}
+            expect(@auth_user.name).to eq 'new user name'
+            expect(@auth_user.email).to eq 'test-1@gmail.com'
+            expect(@auth_user.username).to eq 'newuser'
+            expect(@auth_user.password).to match regex_base64
+          end
+        end
+
+        context 'user returns to website' do
+          let!(:auth_user) { create(:user, email: 'test-1@gmail.com') }
+
+          it 'finds corresponding user' do
+            expect(User.find_for_auth(auth, nil)).to eq auth_user
+          end
+        end
+      end
+    end
+
+    describe '#get_roles' do
+      let(:conf2_organizer_role) { Role.find_by(name: 'organizer', resource: conference2) }
+
+      before do
+        user.update_attributes(role_ids: [organizer_role.id, cfp_role.id, conf2_organizer_role.id])
+      end
+
+      it 'returns hash of role and conference' do
+        expected_hash = {
+          'organizer' => ['oSC16', 'oSC15'],
+          'cfp' => ['oSC16']
+        }
+
+        expect(user.get_roles).to eq expected_hash
+      end
+    end
+
+    describe '#registered' do
+      context 'user has not registered to any conference' do
+        it 'returns None' do
+          expect(user.registered).to eq 'None'
+        end
+      end
+
+      context 'user has registered to conferences' do
+        before do
+          create(:registration, user: user, conference: conference)
+          create(:registration, user: user, conference: conference2)
+        end
+
+        it 'returns registered conferences title' do
+          expect(user.registered).to eq('openSUSE Conference 2016, openSUSE Conference 2015')
+        end
+      end
+    end
+
+    describe '#attended' do
+      context 'user has not attended any conference' do
+        it 'returns None' do
+          expect(user.attended).to eq 'None'
+        end
+      end
+
+      context 'user has attended conferences' do
+        before do
+          create(:registration, user: user, conference: conference, attended: true)
+          create(:registration, user: user, conference: conference2, attended: true)
+        end
+
+        it 'returns attended conferences title' do
+          expect(user.attended).to eq('openSUSE Conference 2016, openSUSE Conference 2015')
+        end
+      end
+    end
+
+    describe '#confirmed?' do
+      context 'confirmed user' do
+        it 'returns true' do
+          expect(user.confirmed?).to eq true
+        end
+      end
+
+      context 'unconfirmed user' do
+        before { user.update_attributes(confirmed_at: nil) }
+
+        it 'returns false' do
+          expect(user.confirmed?).to eq false
+        end
+      end
+    end
+
+    describe 'proposals methods' do
+      let(:submitter) { create(:submitter, user: user) }
+      let(:event1) { create(:event, program: conference.program) }
+      let(:event2) { create(:event, program: conference.program) }
+
+      before do
+        event1.event_users << create(:event_user, user: user, event_role: 'submitter')
+        event2.event_users << create(:event_user, user: user, event_role: 'submitter')
+      end
+
+      describe '#proposals' do
+        it 'returns events submitted by user' do
+          expect(user.proposals(conference)).to match [event1, event2]
+        end
+      end
+
+      describe '#proposal_count' do
+        it 'returns number of events submitted by user' do
+          expect(user.proposal_count(conference)).to eq 2
+        end
+      end
+    end
+  end
+
+  describe 'rolify' do
+    it 'returns the correct role' do
+      expect(user_admin.is_admin).to eq(true)
+      expect(organizer.roles.first).to eq(organizer_role)
+    end
+
+    it 'returns the correct roles' do
+      roles = [organizer_role.id, cfp_role.id]
+      another_user = create(:user, email: 'participant@example.de')
+      another_user.role_ids = roles
+      another_user.save
+
+      expect(another_user.roles.length).to eq(2)
+      expect(another_user.roles[0]).to eq(organizer_role)
+      expect(another_user.roles[1]).to eq(cfp_role)
+    end
+
+    describe '#has_role?' do
+      describe 'when user has a role' do
+        it 'returns true when the user has the role' do
+          user = create(:user, role_ids: organizer_role.id)
+          expect(user.has_role?('organizer', conference)).to be true
+        end
+
+        it 'returns false when the user does not have the role' do
+          user = create(:user, role_ids: cfp_role.id)
+          expect(user.has_role?('organizer', conference)).to be false
+        end
+      end
+
+      it 'returns false when the user does not have a role' do
+        user = create(:user, role_ids: [])
         expect(user.has_role?('organizer', conference)).to be false
       end
     end
-
-    it 'returns false when the user does not have a role' do
-      user = create(:user, role_ids: [])
-      expect(user.has_role?('organizer', conference)).to be false
-    end
   end
 
   describe 'assigns admin attribute' do