From 0c5c06719e1751ef12f5d007e0bd19c8ef606c57 Mon Sep 17 00:00:00 2001 From: WANG QIANG Date: Sat, 16 May 2015 14:37:18 +0800 Subject: [PATCH] Remove useless functions and refactor specs --- app/models/course/group.rb | 30 ------------- app/models/course/group_user.rb | 4 -- spec/models/course/group_spec.rb | 62 ++------------------------- spec/models/course/group_user_spec.rb | 18 +------- spec/models/course_spec.rb | 1 + spec/models/user_spec.rb | 2 + 6 files changed, 8 insertions(+), 109 deletions(-) diff --git a/app/models/course/group.rb b/app/models/course/group.rb index 27b54ba52b5..d0d3d8f27cf 100644 --- a/app/models/course/group.rb +++ b/app/models/course/group.rb @@ -4,34 +4,4 @@ class Course::Group < ActiveRecord::Base has_many :group_users, inverse_of: :group, dependent: :destroy, class_name: Course::GroupUser.name, foreign_key: :course_group_id has_many :users, through: :group_users - - # Add the given user to this group. If the user does not belong to any group, a group user - # will be created for him. Otherwise the existing group user will be updated to belong to - # this group. - # @param [User] The user to be added. The user must have already been enrolled to the course. - def add_user(user) - current_group = user.groups.for_course(course).first - if current_group - current_group.group_users.where(user_id: user.id).first.update(course_group_id: id) - else - group_users.create(user: user, role: role(user)) - end - end - - # Get the role of the user in this group. - # @param [User] The user to be checked. - # @returns [String] The role of the user in this group as a string. If the user is member of the - # group the role will be :manager if the user is enrolled as a staff or :normal otherwise. If - # the user is not part of the group, nil will be returned. - def role_for_user(user) - group_user = group_users.where(user_id: user.id).first - group_user.role if group_user - end - - private - - def role(user) #:nodoc: - return :manager if course.course_users.where(user_id: user.id).first.staff? - :normal - end end diff --git a/app/models/course/group_user.rb b/app/models/course/group_user.rb index 141a027ed2f..77da48508a9 100644 --- a/app/models/course/group_user.rb +++ b/app/models/course/group_user.rb @@ -5,8 +5,4 @@ class Course::GroupUser < ActiveRecord::Base foreign_key: :course_group_id enum role: { normal: 0, manager: 1 } - - def course_user - user.course_users.where(course_id: group.course.id).first - end end diff --git a/spec/models/course/group_spec.rb b/spec/models/course/group_spec.rb index bb9536cf73a..47f744ab12b 100644 --- a/spec/models/course/group_spec.rb +++ b/spec/models/course/group_spec.rb @@ -1,63 +1,7 @@ require 'rails_helper' RSpec.describe Course::Group, type: :model do - it { is_expected.to belong_to(:course).class_name(Course.name) } - - let!(:instance) { create(:instance) } - - with_tenant(:instance) do - let!(:course_owner) { create(:course_user, role: :owner) } - let!(:course) { course_owner.course } - let!(:owner) { course_owner.user } - let!(:course_student) { create(:course_user, course: course, role: :student) } - let!(:student) { course_student.user } - let!(:group) { create(:course_group, course: course) } - let!(:group2) { create(:course_group, course: course) } - - before do - User.stamper = owner - end - - describe '#add_user' do - subject { group.add_user(owner) } - - context 'user is not a member of any group' do - it 'creates a new group user for the user' do - expect { subject }.to change(Course::GroupUser, :count).by(1) - owner.reload - expect(Course::GroupUser.last).to eq(owner.group_users.first) - end - end - - context 'user is a member of another group' do - before do - create(:course_group_user, user: owner, group: group2) - end - - it 'removes the user from the previous group and assign him to the current one' do - expect(owner.groups.last).to eq(group2) - expect { subject }.not_to change(Course::GroupUser, :count) - owner.reload - expect(owner.groups.last).to eq(group) - end - end - end - - describe '#role_for_user' do - before do - create(:course_group_user, group: group, user: owner, role: :manager) - end - context 'user is a group member' do - it "returns the user's role in the group" do - expect(group.role_for_user(owner)).to eq('manager') - end - end - - context 'user is a not a group member' do - it 'returns nil' do - expect(group.role_for_user(student)).to be(nil) - end - end - end - end + it { is_expected.to belong_to(:course).inverse_of(:groups) } + it { is_expected.to have_many(:group_users).inverse_of(:group).dependent(:destroy) } + it { is_expected.to have_many(:users).through(:group_users) } end diff --git a/spec/models/course/group_user_spec.rb b/spec/models/course/group_user_spec.rb index 19a6af17c11..9de27325e16 100644 --- a/spec/models/course/group_user_spec.rb +++ b/spec/models/course/group_user_spec.rb @@ -1,20 +1,6 @@ require 'rails_helper' RSpec.describe Course::GroupUser, type: :model do - it { is_expected.to belong_to(:user).class_name(User.name) } - it { is_expected.to belong_to(:group).class_name(Course::Group.name) } - - let!(:instance) { create(:instance) } - with_tenant(:instance) do - let!(:course_student) { create(:course_user) } - let!(:course) { course_student.course } - let!(:group) { create(:course_group, course: course) } - let!(:group_student) { create(:course_group_user, group: group, user: course_student.user) } - - describe '#course_user' do - it 'returns the course user that belongs to the user and the course' do - expect(group_student.course_user).to eq(course_student) - end - end - end + it { is_expected.to belong_to(:user).inverse_of(:group_users) } + it { is_expected.to belong_to(:group).inverse_of(:group_users) } end diff --git a/spec/models/course_spec.rb b/spec/models/course_spec.rb index 7562b3d9178..086a002a108 100644 --- a/spec/models/course_spec.rb +++ b/spec/models/course_spec.rb @@ -8,6 +8,7 @@ it { is_expected.to have_many(:users).through(:course_users) } it { is_expected.to have_many(:announcements).inverse_of(:course).dependent(:destroy) } it { is_expected.to have_many(:levels).inverse_of(:course).dependent(:destroy) } + it { is_expected.to have_many(:groups).inverse_of(:course).dependent(:destroy) } it { is_expected.to validate_presence_of(:title) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d1e824260b8..0b3a0db4bf4 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -11,6 +11,8 @@ it { is_expected.to have_many(:instances).through(:instance_users) } it { is_expected.to have_many(:course_users).inverse_of(:user).dependent(:destroy) } it { is_expected.to have_many(:courses).through(:course_users) } + it { is_expected.to have_many(:group_users).inverse_of(:user).dependent(:destroy) } + it { is_expected.to have_many(:groups).through(:group_users) } describe '#email' do context 'when the user has no email addresses' do