Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove references to Follower.user_id. #14115

Merged
merged 3 commits into from Apr 4, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion dashboard/app/controllers/admin_search_controller.rb
Expand Up @@ -31,7 +31,7 @@ def find_students
end
if teachers.first
array_of_student_ids = Follower.
where(user_id: teachers.first[:id]).pluck('student_user_id').to_a
where(user: teachers.first).pluck('student_user_id').to_a
users = users.where(id: array_of_student_ids)
end
end
Expand Down
2 changes: 1 addition & 1 deletion dashboard/app/controllers/transfers_controller.rb
Expand Up @@ -80,7 +80,7 @@ def create
follower_same_user_teacher.update_attributes!(section_id: new_section.id)
else
unless student.followeds.exists?(section_id: new_section.id)
student.followeds.create!(user_id: new_section.user_id, section: new_section)
student.followeds.create!(user: new_section.user, section: new_section)
end

unless stay_enrolled_in_current_section
Expand Down
2 changes: 1 addition & 1 deletion dashboard/app/models/follower.rb
Expand Up @@ -32,7 +32,7 @@ def cannot_follow_yourself
end

def teacher_must_be_teacher
errors.add(:user_id, "must be a teacher") unless user.user_type == User::TYPE_TEACHER
errors.add(:user, "must be a teacher") unless user.user_type == User::TYPE_TEACHER
end

validate :cannot_follow_yourself, :teacher_must_be_teacher
Expand Down
13 changes: 3 additions & 10 deletions dashboard/app/models/user.rb
Expand Up @@ -729,7 +729,7 @@ def student_of_authorized_teacher?
end

def student_of?(teacher)
followeds.find_by_user_id(teacher.id).present?
teachers.include? teacher
end

def locale
Expand Down Expand Up @@ -1207,7 +1207,7 @@ def can_edit_password?
end

def section_for_script(script)
followeds.collect(&:section).find {|section| section.script_id == script.id}
sections_as_student.find {|section| section.script_id == script.id}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tangentially related, this line change could be part of a separate PR.

end

# Returns the version of our Terms of Service we consider the user as having
Expand All @@ -1218,14 +1218,7 @@ def terms_version
if teacher?
return terms_of_service_version
end
# As of August 2016, it may be the case that the `followed` exists but
# `followed.user` does not as the result of user deletion. In this case, we
# ignore any terms of service versions associated with deleted teacher
# accounts.
followeds.
collect {|followed| followed.user.try(:terms_of_service_version)}.
compact.
max
teachers.map(&:terms_of_service_version).try(:compact).try(:max)
end

# Returns whether the user has accepted the latest major version of the Terms of Service
Expand Down
4 changes: 2 additions & 2 deletions dashboard/test/controllers/script_levels_controller_test.rb
Expand Up @@ -15,7 +15,7 @@ class ScriptLevelsControllerTest < ActionController::TestCase
@teacher = create :teacher
@admin = create :admin
@section = create :section, user_id: @teacher.id
Follower.create!(section_id: @section.id, student_user_id: @student.id, user_id: @teacher.id)
Follower.create!(section_id: @section.id, student_user_id: @student.id, user: @teacher)

@custom_script = create(:script, name: 'laurel', hideable_stages: true)
@custom_stage_1 = create(:stage, script: @custom_script, name: 'Laurel Stage 1', absolute_position: 1, relative_position: '1')
Expand Down Expand Up @@ -1248,7 +1248,7 @@ def assert_caching_enabled(cache_control_header, max_age, proxy_max_age)

def put_student_in_section(student, teacher, script)
section = create :section, user_id: teacher.id, script_id: script.id
Follower.create!(section_id: section.id, student_user_id: student.id, user_id: teacher.id)
Follower.create!(section_id: section.id, student_user_id: student.id, user: teacher)
section
end

Expand Down
4 changes: 2 additions & 2 deletions dashboard/test/controllers/transfers_controller_test.rb
Expand Up @@ -137,7 +137,7 @@ class TransfersControllerTest < ActionController::TestCase
test "multiple students can be transferred" do
new_student = create(:student)
Follower.create!(
user_id: @teacher.id,
user: @teacher,
student_user: new_student,
section: @word_section
)
Expand All @@ -153,7 +153,7 @@ class TransfersControllerTest < ActionController::TestCase
test "students can be transferred to other teachers if they already belong to a section belonging to the other teacher" do
already_enrolled_section = create(:section, user: @other_teacher, login_type: 'word')
Follower.create!(
user_id: @other_teacher.id,
user: @other_teacher,
student_user: @word_student,
section: already_enrolled_section
)
Expand Down
4 changes: 2 additions & 2 deletions dashboard/test/models/follower_test.rb
Expand Up @@ -9,8 +9,8 @@ class FollowerTest < ActiveSupport::TestCase
test "cannot follow yourself" do
assert_does_not_create(Follower) do
follower = Follower.create(
user_id: @laurel.id,
student_user_id: @laurel.id,
user: @laurel,
student_user: @laurel,
section: @laurel_section
)
refute follower.valid?
Expand Down
2 changes: 1 addition & 1 deletion dashboard/test/models/user_test.rb
Expand Up @@ -1477,7 +1477,7 @@ def create_user_with_username(username)
teacher = create :teacher
section = create :section, user_id: teacher.id

follow = Follower.create!(section_id: section.id, student_user_id: student.id, user_id: teacher.id)
follow = Follower.create!(section_id: section.id, student_user_id: student.id, user: teacher)

teacher.reload
student.reload
Expand Down
10 changes: 6 additions & 4 deletions pegasus/helper_modules/dashboard.rb
Expand Up @@ -82,10 +82,11 @@ def select(*keys)
# @return [Array[Integer]] the subset of other_user_ids that are followeds
# of the user encapsulated by this class.
def get_followed_bys(other_user_ids)
Dashboard.db[:followers].
Dashboard.db[:sections].
join(:followers, section_id: :sections__id).
join(:users, id: :followers__student_user_id).
where(sections__user_id: id).
where(followers__student_user_id: other_user_ids).
where(followers__user_id: id).
where(users__deleted_at: nil, followers__deleted_at: nil).
select_map(:followers__student_user_id)
end
Expand All @@ -94,10 +95,11 @@ def get_followed_bys(other_user_ids)
# @return [Boolean] whether other_user_id is a followed of the user
# encapsulated by this class.
def followed_by?(other_user_id)
Dashboard.db[:followers].
Dashboard.db[:sections].
join(:followers, section_id: :sections__id).
join(:users, id: :followers__student_user_id).
where(sections__user_id: id).
where(followers__student_user_id: other_user_id).
where(followers__user_id: id).
where(users__deleted_at: nil, followers__deleted_at: nil).
any?
end
Expand Down
24 changes: 12 additions & 12 deletions pegasus/helpers/section_api_helpers.rb
Expand Up @@ -10,11 +10,12 @@
class DashboardStudent
# Returns all users who are followers of the user with ID user_id.
def self.fetch_user_students(user_id)
Dashboard.db[:users].
join(:followers, student_user_id: :users__id).
join(Sequel.as(:users, :users_students), id: :followers__student_user_id).
where(followers__user_id: user_id, followers__deleted_at: nil).
where(users_students__deleted_at: nil).
Dashboard.db[:sections].
join(:followers, section_id: :sections__id).
join(:users, id: :followers__student_user_id).
where(sections__user_id: user_id).
where(followers__deleted_at: nil).
where(users__deleted_at: nil).
select(*fields).
all
end
Expand Down Expand Up @@ -105,12 +106,12 @@ def self.fetch_if_allowed_array(ids, dashboard_user_id)
end

def self.update_if_allowed(params, dashboard_user_id)
user_to_update = Dashboard.db[:users].
where(id: params[:id], deleted_at: nil)
user_to_update = Dashboard.db[:users].where(id: params[:id], deleted_at: nil)
return if user_to_update.empty?
return if Dashboard.db[:followers].
where(student_user_id: params[:id], user_id: dashboard_user_id).
where(deleted_at: nil).
return if Dashboard.db[:sections].
join(:followers, section_id: :sections__id).
where(sections__user_id: dashboard_user_id).
where(followers__student_user_id: params[:id], followers__deleted_at: nil).
empty?

fields = {updated_at: DateTime.now}
Expand Down Expand Up @@ -383,9 +384,8 @@ def add_student(student)
created_at = DateTime.now
Dashboard.db[:followers].insert(
{
user_id: @row[:teacher_id],
student_user_id: student_id,
section_id: @row[:id],
student_user_id: student_id,
created_at: created_at,
updated_at: created_at,
}
Expand Down
3 changes: 0 additions & 3 deletions pegasus/test/fixtures/fake_dashboard.rb
Expand Up @@ -85,17 +85,14 @@ module FakeDashboard
#
FOLLOWERS = [
{
user_id: TEACHER[:id],
student_user_id: STUDENT[:id],
section_id: SECTION_NORMAL[:id]
},
{
user_id: TEACHER_WITH_DELETED[:id],
student_user_id: DELETED_STUDENT[:id],
section_id: SECTION_DELETED_FOLLOWERS[:id]
},
{
user_id: TEACHER_WITH_DELETED[:id],
student_user_id: SELF_STUDENT[:id],
section_id: SECTION_DELETED_FOLLOWERS[:id],
deleted_at: '2016-01-01 00:01:02'
Expand Down
4 changes: 2 additions & 2 deletions pegasus/test/test_v2_section_routes.rb
Expand Up @@ -205,15 +205,15 @@ class V2SectionRoutesTest < Minitest::Test
Dashboard.db.transaction(rollback: :always) do
before_timestamp = Dashboard.db[:followers].
where(
user_id: FakeDashboard::TEACHER_WITH_DELETED[:id],
section_id: FakeDashboard::SECTION_DELETED_FOLLOWERS[:id],
student_user_id: FakeDashboard::SELF_STUDENT[:id]
).
select_map(:deleted_at)
@pegasus.post "/v2/sections/#{FakeDashboard::SECTION_DELETED_FOLLOWERS[:id]}/delete"
assert_equal 204, @pegasus.last_response.status
after_timestamp = Dashboard.db[:followers].
where(
user_id: FakeDashboard::TEACHER_WITH_DELETED[:id],
section_id: FakeDashboard::SECTION_DELETED_FOLLOWERS[:id],
student_user_id: FakeDashboard::SELF_STUDENT[:id]
).
select_map(:deleted_at)
Expand Down