Adds data constraints in package model.

app/models/package.rb

@@ -8,7 +8,14 @@ class Package < ActiveRecord::Base
   attr_accessor :url_substring, :link_text, :provider_name, :collection_name, :access_type, :no_ftp_search
 
   validates :record_id, uniqueness: true
-  validates :package_name, :vendor_name, :data_pickup_type, :package_status, presence: true
+  validates :package_id, :package_name, :vendor_name, :data_pickup_type, :package_status, presence: true
+  validates :package_id, format: { with: /\A[a-z]{2}\z/, message: 'ID only 2-character alpha code allowed' },
+                         uniqueness: true
+  validate :check_if_package_id_changed, on: :update
+  validate :needs_afs_path, on: %i(create update)
+  validate :needs_ftp_info, on: %i(create update)
+  validate :needs_put_file_loc, on: %i(create update)
+  validate :check_match_opts, on: %i(create update)
 
   COLUMNS = {
     'package_status' => 'Package status',
@@ -84,4 +91,53 @@ def timestamp_attributes_for_create
   def timestamp_attributes_for_update
     super << :date_modified
   end
+
+  def check_if_package_id_changed
+    errors.add(:base, 'Package ID is marked as readonly') if package_id_changed?
+  end
+
+  def use_afs?
+    data_pickup_type.include?('AFS')
+  end
+
+  def use_ftp?
+    data_pickup_type.include?('FTP')
+  end
+
+  def needs_afs_path
+    if use_afs?
+      errors.add(:base, 'AFS directory path is empty but data pickup type is AFS.') unless afs_path.present?
+    else
+      nil
+    end
+  end
+
+  def needs_ftp_info
+    if use_ftp?
+      errors.add(:base, 'FTP server is empty but data pickup type is FTP.') unless ftp_server.present?
+      errors.add(:base, 'FTP user is empty but data pickup type is FTP.') unless ftp_user.present?
+      errors.add(:base, 'FTP password is empty but data pickup type is FTP.') unless ftp_password.present?
+      errors.add(:base, 'FTP remote directory is empty but data pickup type is FTP.') unless ftp_directory.present?
+    else
+      nil
+    end
+  end
+
+  def needs_put_file_loc
+    if use_afs? && use_ftp?
+      errors.add(:base, 'FTP to AFS download directory path is empty but data pickup type is FTP to AFS.') unless put_file_loc.present?
+    else
+      nil
+    end
+  end
+
+  def needs_match_options?
+    (proc_type.include?('newmerge') || proc_type.include?('mergeonly')) if proc_type.present?
+  end
+
+  def check_match_opts
+    if needs_match_options?
+      errors.add(:base, "#{COLUMNS.fetch('match_opts')} is empty but processing rule needs it.") unless match_opts.present?
+    end
+  end
 end

app/views/packages/_form.html.erb

@@ -1,7 +1,7 @@
 <%= form_for @package, class: 'form-group' do |f| %>
   <% if @package.errors.any? %>
-    <div id="error_explanation">
-      <h2><%= pluralize(@package.errors.count, "error") %> prohibited this package from being saved:</h2>
+    <div id="error-explanation">
+      <h4><%= pluralize(@package.errors.count, "error") %> prohibited this package from being saved:</h4>
 
       <ul>
       <% @package.errors.full_messages.each do |message| %>

spec/controllers/packages_controller_spec.rb

@@ -6,7 +6,7 @@
   end
   let(:valid_attributes) do
     { package_name: 'different name', package_status: 'Inactive',
-      vendor_name: 'different vend', data_pickup_type: 'some other' }
+      vendor_name: 'different vend', data_pickup_type: 'AFS', afs_path: 'somedir' }
   end
   let(:invalid_attributes) { { package_name: '', package_status: 'Active', vendor_name: '', data_pickup_type: '' } }
   describe 'GET #index' do
@@ -25,13 +25,15 @@
     end
   end
   describe 'POST #create' do
+    before do
+      valid_attributes.update(package_id: 'id')
+    end
     context 'with valid params' do
       it 'creates a new Package' do
-        expect { post(:create, package: valid_attributes) }.to change(Package, :count).by(1)
+        expect { post :create, package: valid_attributes }.to change(Package, :count).by(1)
       end
       it 'redirects to the created package' do
-        post :create, package: valid_attributes
-        expect(response).to redirect_to(Package.last)
+        expect(post(:create, package: valid_attributes)).to redirect_to(Package.last)
       end
     end
     context 'with invalid params' do

spec/factories/packages.rb

@@ -3,9 +3,9 @@
     f.package_name 'Some package name'
     f.vendor_name 'Some vnd name'
     f.record_id 1
-    f.package_id 'something'
+    f.package_id 'np'
     f.package_status 'something'
-    f.data_pickup_type 'something'
+    f.data_pickup_type 'AFS'
     f.afs_path 'something'
     f.ftp_server 'something'
     f.ftp_user 'something'

spec/models/package_spec.rb

@@ -0,0 +1,39 @@
+require 'rails_helper'
+
+RSpec.describe Package, type: :model do
+  before do
+    @package = FactoryBot.create(:package)
+  end
+  it 'has a valid factory' do
+    expect(@package).to be_valid
+  end
+  it 'has an immutable package_id' do
+    @package.update(package_id: 'id')
+    expect { @package.save! }.to raise_error(ActiveRecord::RecordInvalid)
+  end
+  it 'adds an error when afs_path is not present for AFS data_pickup_type' do
+    @package.update(afs_path: '')
+    expect { @package.save! }.to raise_error(ActiveRecord::RecordInvalid)
+    expect(@package).to_not be_valid
+  end
+  it 'adds an error when FTP information is not present for FTP data_pickup_type' do
+    @package.update(data_pickup_type: 'FTP')
+    @package.update(ftp_server: '')
+    expect { @package.save! }.to raise_error(ActiveRecord::RecordInvalid)
+  end
+  it 'adds an error when AFS and FTP information is not present for FTP to AFS data_pickup_type' do
+    @package.update(data_pickup_type: 'FTP to AFS')
+    @package.update(afs_path: '')
+    @package.update(ftp_server: '')
+    expect { @package.save! }.to raise_error(ActiveRecord::RecordInvalid)
+  end
+  it 'adds an error when FTP to AFS download directory path is not present for FTP to AFS data_pickup_type' do
+    @package.update(data_pickup_type: 'FTP to AFS')
+    @package.update(put_file_loc: '')
+    expect { @package.save! }.to raise_error(ActiveRecord::RecordInvalid)
+  end
+  it 'adds an error when match_opts is empty but proc_type is newmerge or mergeonly' do
+    @package.update(match_opts: nil)
+    expect { @package.save! }.to raise_error(ActiveRecord::RecordInvalid)
+  end
+end