From 1d5017641f04cb0ae7f209103032016636d3f236 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 1 May 2014 16:34:06 -0400 Subject: [PATCH] Remove email from User model and API It was never actually used for anything except to enforce uniqueness, which should be done by the LMS. --- Rakefile | 2 +- api/users.rb | 3 +-- edx_specs/helpers.rb | 1 - lib/tasks/benchmark.rake | 2 +- models/user.rb | 3 --- scripts/db/migrate-006-remove-email.js | 1 + scripts/db/revert-migrate-006-remove-email.js | 1 + spec/api/notifications_spec.rb | 6 +++--- spec/api/user_spec.rb | 11 ++++------- spec/models/at_user_observer_spec.rb | 4 ++-- spec/spec_helper.rb | 2 +- 11 files changed, 15 insertions(+), 21 deletions(-) create mode 100644 scripts/db/migrate-006-remove-email.js create mode 100644 scripts/db/revert-migrate-006-remove-email.js diff --git a/Rakefile b/Rakefile index 8c2c84d..66b4e5c 100644 --- a/Rakefile +++ b/Rakefile @@ -34,7 +34,7 @@ task :environment do end def create_test_user(id) - User.create!(external_id: id, username: "user#{id}", email: "user#{id}@test.com") + User.create!(external_id: id, username: "user#{id}") end Dir.glob('lib/tasks/*.rake').each { |r| import r } diff --git a/api/users.rb b/api/users.rb index 8403141..4109fd3 100644 --- a/api/users.rb +++ b/api/users.rb @@ -3,7 +3,6 @@ require 'new_relic/agent/method_tracer' post "#{APIPREFIX}/users" do user = User.new(external_id: params["id"]) user.username = params["username"] - user.email = params["email"] user.save if user.errors.any? error 400, user.errors.full_messages.to_json @@ -67,7 +66,7 @@ end put "#{APIPREFIX}/users/:user_id" do |user_id| user = User.find_or_create_by(external_id: user_id) - user.update_attributes(params.slice(*%w[username email default_sort_key])) + user.update_attributes(params.slice(*%w[username default_sort_key])) if user.errors.any? error 400, user.errors.full_messages.to_json else diff --git a/edx_specs/helpers.rb b/edx_specs/helpers.rb index 06143b4..1cfc676 100644 --- a/edx_specs/helpers.rb +++ b/edx_specs/helpers.rb @@ -1,7 +1,6 @@ def log_in visit "/" click_link "Log In" - fill_in "email", with: "student@student.com" fill_in "password", with: "student" click_button "Access My Courses" wait_until { page.find('.dashboard') } diff --git a/lib/tasks/benchmark.rake b/lib/tasks/benchmark.rake index b0c39c8..5709569 100644 --- a/lib/tasks/benchmark.rake +++ b/lib/tasks/benchmark.rake @@ -21,7 +21,7 @@ namespace :benchmark do x.report "create users" do (1..USERS).each do |user_id| - data = { id: user_id, username: "user#{user_id}", email: "user#{user_id}@test.com" } + data = { id: user_id, username: "user#{user_id}" } RestClient.post "#{PREFIX}/users", data end end diff --git a/models/user.rb b/models/user.rb index df677c5..2abddf0 100644 --- a/models/user.rb +++ b/models/user.rb @@ -7,7 +7,6 @@ class User field :_id, type: String, default: -> { external_id } field :external_id, type: String field :username, type: String - field :email, type: String field :default_sort_key, type: String, default: "date" embeds_many :read_states @@ -18,10 +17,8 @@ class User validates_presence_of :external_id validates_presence_of :username - validates_presence_of :email validates_uniqueness_of :external_id validates_uniqueness_of :username - validates_uniqueness_of :email index( {external_id: 1}, {unique: true, background: true} ) diff --git a/scripts/db/migrate-006-remove-email.js b/scripts/db/migrate-006-remove-email.js new file mode 100644 index 0000000..7b9a0b2 --- /dev/null +++ b/scripts/db/migrate-006-remove-email.js @@ -0,0 +1 @@ +db.users.dropIndex({email: 1}) diff --git a/scripts/db/revert-migrate-006-remove-email.js b/scripts/db/revert-migrate-006-remove-email.js new file mode 100644 index 0000000..0f80cec --- /dev/null +++ b/scripts/db/revert-migrate-006-remove-email.js @@ -0,0 +1 @@ +db.users.ensureIndex({email: 1}, {background: true}) diff --git a/spec/api/notifications_spec.rb b/spec/api/notifications_spec.rb index c4d6a83..afc734f 100644 --- a/spec/api/notifications_spec.rb +++ b/spec/api/notifications_spec.rb @@ -11,7 +11,7 @@ describe "app" do describe "POST /api/v1/notifications" do it "returns notifications by class and user" do start_time = Time.now - user = User.create(:email => "test@example.com",:external_id => 1,:username => "example") + user = User.create(:external_id => 1,:username => "example") commentable = Commentable.new("question_1") random_string = (0...8).map{ ('a'..'z').to_a[rand(26)] }.join thread = CommentThread.new(title: "Test title", body: "elephant otter", course_id: "1", commentable_id: commentable.id, comments_text_dummy: random_string) @@ -42,7 +42,7 @@ describe "app" do # first make a dummy thread and comment and a subscription commentable = Commentable.new("question_1") - user = User.create(:email => "test@example.com",:external_id => 1,:username => "example") + user = User.create(:external_id => 1,:username => "example") random_string = (0...8).map{ ('a'..'z').to_a[rand(26)] }.join thread = CommentThread.new(title: "Test title", body: "elephant otter", course_id: "1", commentable_id: commentable.id, comments_text_dummy: random_string) @@ -85,7 +85,7 @@ describe "app" do start_time = Date.today - 100.days - user = User.create(:email => "test@example.com",:external_id => 1,:username => "example") + user = User.create(:external_id => 1,:username => "example") sleep 1 diff --git a/spec/api/user_spec.rb b/spec/api/user_spec.rb index 14a00dc..6598bc0 100644 --- a/spec/api/user_spec.rb +++ b/spec/api/user_spec.rb @@ -11,18 +11,15 @@ describe "app" do end describe "POST /api/v1/users" do it "creates a user" do - post "/api/v1/users", id: "100", username: "user100", email: "user100@test.com" + post "/api/v1/users", id: "100", username: "user100" last_response.should be_ok user = User.find_by(external_id: "100") user.username.should == "user100" - user.email.should == "user100@test.com" end - it "returns error when id / username / email already exists" do - post "/api/v1/users", id: "1", username: "user100", email: "user100@test.com" + it "returns error when id / username already exists" do + post "/api/v1/users", id: "1", username: "user100" last_response.status.should == 400 - post "/api/v1/users", id: "100", username: "user1", email: "user100@test.com" - last_response.status.should == 400 - post "/api/v1/users", id: "100", username: "user100", email: "user1@test.com" + post "/api/v1/users", id: "100", username: "user1" last_response.status.should == 400 end end diff --git a/spec/models/at_user_observer_spec.rb b/spec/models/at_user_observer_spec.rb index 64791c4..dd60551 100644 --- a/spec/models/at_user_observer_spec.rb +++ b/spec/models/at_user_observer_spec.rb @@ -23,8 +23,8 @@ require 'spec_helper' #what is the 'at' symbol doing there? @dementrock #""" # User.delete_all -# User.create!(external_id: "1", username: "tom", email: "tom@test.com") -# User.create!(external_id: "2", username: "pi314", email: "pi314@test.com") +# User.create!(external_id: "1", username: "tom") +# User.create!(external_id: "2", username: "pi314") # end # # describe "#get_marked_text(text)" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 26047cd..51ce436 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -52,7 +52,7 @@ def parse(text) end def create_test_user(id) - User.create!(external_id: id.to_s, username: "user#{id}", email: "user#{id}@test.com") + User.create!(external_id: id.to_s, username: "user#{id}") end def init_without_subscriptions