Merge pull request #386 from sul-dlss/undo-user-email

Undo user email

app/models/user.rb

@@ -1,9 +1,9 @@
 class User < ApplicationRecord
   devise :remote_user_authenticatable # We don't want other (default) Devise modules
   has_many :bundle_contexts, dependent: :destroy
-  validates :email, presence: true, uniqueness: true
+  validates :sunet_id, presence: true, uniqueness: true
 
-  def sunet_id
-    email.split('@').first
+  def email
+    sunet_id.include?('@') ? sunet_id : "#{sunet_id}@stanford.edu"
   end
 end

config/initializers/devise.rb

@@ -58,7 +58,7 @@
   # session. If you need permissions, you should implement that in a before filter.
   # You can also supply a hash where the value is a boolean determining whether
   # or not authentication should be aborted when the value is not present.
-  config.authentication_keys = [:email]
+  config.authentication_keys = [:sunet_id]
 
   # Configure parameters from the request object used for authentication. Each entry
   # given should be a request method and it will automatically be passed to the

db/schema.rb

@@ -35,10 +35,10 @@
   end
 
   create_table "users", force: :cascade do |t|
-    t.string "email", null: false
+    t.string "sunet_id", null: false
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
-    t.index ["email"], name: "index_users_on_email", unique: true
+    t.index ["sunet_id"], name: "index_users_on_sunet_id", unique: true
   end
 
 end

spec/controllers/bundle_contexts_controller_spec.rb

@@ -36,7 +36,7 @@
 
     context 'POST create' do
       context 'Valid Parameters' do
-        let(:output_dir) { "#{Settings.job_output_parent_dir}/#{subject.current_user.email}/SMPL-multimedia" }
+        let(:output_dir) { "#{Settings.job_output_parent_dir}/#{subject.current_user.sunet_id}/SMPL-multimedia" }
 
         before { Dir.delete(output_dir) if Dir.exist?(output_dir) }
 

spec/factories/users.rb

@@ -1,5 +1,5 @@
 FactoryBot.define do
   factory :user do
-    sequence(:email) { |n| "fake_#{n}@stanford.edu" }
+    sequence(:sunet_id) { |n| "fake_#{n}@stanford.edu" } # TODO: migrate this to an `email` field
   end
 end

spec/models/bundle_context_spec.rb

@@ -103,7 +103,7 @@
 
   describe 'output_dir' do
     it 'returns "Settings.job_output_parent_dir/user_id/project_name"' do
-      expect(bc.output_dir).to eq "#{Settings.job_output_parent_dir}/#{bc.user.email}/#{bc.project_name}"
+      expect(bc.output_dir).to eq "#{Settings.job_output_parent_dir}/#{bc.user.sunet_id}/#{bc.project_name}"
     end
   end
 

spec/models/user_spec.rb

@@ -1,21 +1,21 @@
 RSpec.describe User, type: :model do
-  subject(:user) { build(:user, email: 'jdoe@stanford.edu') }
+  subject(:user) { build(:user, sunet_id: 'jdoe@stanford.edu') }
 
   context 'validation' do
     it 'is not valid unless it has all required attributes' do
       expect(User.new).not_to be_valid
       expect(user).to be_valid
     end
 
-    it { is_expected.to validate_uniqueness_of(:email).ignoring_case_sensitivity }
-    it { is_expected.to validate_presence_of(:email) }
+    it { is_expected.to validate_uniqueness_of(:sunet_id) }
+    it { is_expected.to validate_presence_of(:sunet_id) }
     it { is_expected.to have_many(:bundle_contexts) }
 
-    describe 'enforces unique constraint on email' do
+    describe 'enforces unique constraint on sunet_id' do
       before { user.save! }
 
       it 'at model level' do
-        expect { described_class.create!(email: user.email) }.to raise_error(ActiveRecord::RecordInvalid)
+        expect { described_class.create!(sunet_id: user.sunet_id) }.to raise_error(ActiveRecord::RecordInvalid)
       end
       it 'at db level' do
         expect { user.dup.save!(validate: false) }.to raise_error(ActiveRecord::RecordNotUnique)
@@ -24,12 +24,12 @@
   end
 
   context 'email address' do
-    it "returns the user's email address" do
+    it "returns the user's email address if the sunet_id is already an address" do
       expect(user.email).to eq('jdoe@stanford.edu')
     end
 
-    it "returns the user's sunet id" do
-      expect(user.sunet_id).to eq('jdoe')
+    it 'returns a stanford.edu email address if the sunet_id is not an email address' do
+      expect { user.sunet_id = 'jdoe' }.not_to change(user, :email).from('jdoe@stanford.edu')
     end
   end
 end

spec/services/discovery_report_spec.rb

@@ -92,7 +92,7 @@
         bundle_dir: 'spec/test_data/images_jp2_tif',
         staging_style_symlink: false,
         content_metadata_creation: 0,
-        user: build(:user, email: 'jdoe@stanford.edu')
+        user: build(:user, sunet_id: 'jdoe@stanford.edu')
       }
     end
     let(:bundle_context) { BundleContext.new(bc_params) }

spec/support/bundle_setup.rb

@@ -22,6 +22,6 @@ def bundle_context_from_hash(proj)
     content_structure: hash['project_style']['content_structure'],
     bundle_dir: hash['bundle_dir'],
     content_metadata_creation: cmc,
-    user: build(:user, email: 'Jdoe@stanford.edu')
+    user: build(:user, sunet_id: 'Jdoe@stanford.edu')
   )
 end