Merge pull request #141 from dpn-admin/fix/137/scopes

Redesign adapters to map belongs_to associations to objects, not their IDs

app/adapters/adapter.rb

@@ -66,18 +66,17 @@ def hidden_field(model_field)
   def map_belongs_to(model_field, public_field, options = {})
     model_class = options[:model_class] || model_field.to_s.classify.constantize
     sub_method = options[:sub_method] || public_field
-    model_field_id = :"#{model_field}_id"
 
     unless options[:only] == :to
       map_from_public public_field do |value|
         record = model_class.send(:"find_by_#{sub_method}", value)
-        {model_field_id => record ? record.id : nil}
+        {model_field => record ? record : model_class.new(sub_method => value)}
       end
     end
 
     unless options[:only] == :from
-      map_to_public model_field_id do |id|
-        {public_field => model_class.find_by(id: id).send(sub_method)}
+      map_to_public model_field do |record|
+        {public_field => record.send(sub_method)}
       end
     end
   end

app/controllers/bags_controller.rb

@@ -15,9 +15,9 @@ class BagsController < ApplicationController
   def index
     @bags = Bag.updated_after(params[:after])
       .updated_before(params[:before])
-      .with_admin_node_id(params[:admin_node_id])
-      .with_ingest_node_id(params[:ingest_node_id])
-      .with_member_id(params[:member_id])
+      .with_admin_node(params[:admin_node])
+      .with_ingest_node(params[:ingest_node])
+      .with_member(params[:member])
       .with_bag_type(params[:type])
       .replicated_by(params[:replicating_nodes])
       .order(parse_ordering(params[:order_by]))
@@ -48,7 +48,7 @@ def create
       else
         Bag.new
       end
-      if @bag.update_with_associations(create_params(params))
+      if @bag.update_with_associations(params)
         render "shared/create", status: 201
       else
         render "shared/errors", status: 400
@@ -60,7 +60,7 @@ def create
   def update
     @bag = Bag.find_by_uuid!(params[:uuid])
 
-    if @bag.update_with_associations(update_params(params))
+    if @bag.update_with_associations(params)
       render "shared/update", status: 200
     else
       render "shared/errors", status: 400
@@ -74,21 +74,4 @@ def destroy
     render nothing: true, status: 204
   end
 
-
-  private
-
-
-  def create_params(params)
-    new_params = params.permit(Bag.attribute_names)
-    new_params.merge! params.slice(
-      :replicating_nodes, :version_family,
-      :rights_bags, :interpretive_bags)
-    return new_params
-  end
-
-  def update_params(params)
-    create_params(params)
-  end
-
-
 end

app/controllers/fixity_checks_controller.rb

@@ -16,8 +16,8 @@ def index
     @fixity_checks = FixityCheck.created_after(params[:after])
       .created_before(params[:before])
       .with_success(params[:success])
-      .with_node_id(params[:node_id])
-      .with_bag_id(params[:bag_id])
+      .with_node(params[:node])
+      .with_bag(params[:bag])
       .latest_only(convert_bool(params[:latest]))
       .page(@page)
       .per(@page_size)
@@ -41,7 +41,9 @@ def create
 
   private
   def create_params(params)
-    params.permit(FixityCheck.attribute_names)
+    params
+      .permit(:fixity_check_id, :fixity_at, :success, :created_at)
+      .merge(params.slice(:bag, :node))
   end
 
 end

app/controllers/ingests_controller.rb

@@ -15,7 +15,7 @@ class IngestsController < ApplicationController
   def index
     @ingests = Ingest.created_after(params[:after])
       .created_before(params[:before])
-      .with_bag_id(params[:bag_id])
+      .with_bag(params[:bag])
       .with_ingested(params[:ingested])
       .latest_only(convert_bool(params[:latest]))
       .page(@page)
@@ -39,8 +39,13 @@ def create
 
 
   private
+  SCALAR_PARAMS = [:ingest_id, :ingested, :created_at]
+  ASSOCIATED_PARAMS = [:bag, :nodes]
+
   def create_params(params)
-    params.permit(Ingest.attribute_names)
+    params
+      .permit(SCALAR_PARAMS)
+      .merge(params.slice(*ASSOCIATED_PARAMS))
   end
 
 end

app/controllers/message_digests_controller.rb

@@ -15,7 +15,7 @@ class MessageDigestsController < ApplicationController
   def index
     @message_digests = MessageDigest.created_after(params[:after])
       .created_before(params[:before])
-      .with_bag_id(params[:bag_id])
+      .with_bag(params[:bag])
       .order(parse_ordering(params[:order_by]))
       .page(@page)
       .per(@page_size)
@@ -26,15 +26,15 @@ def index
 
   def show
     @message_digest = MessageDigest.find_by!(
-      bag_id: params[:bag_id],
-      fixity_alg_id: params[:fixity_alg_id])
+      bag_id: params[:bag]&.id,
+      fixity_alg_id: params[:fixity_alg]&.id)
     render "shared/show", status: 200
   end
 
   def create
     existing = MessageDigest.find_by(
-      bag_id: params[:bag_id],
-      fixity_alg_id: params[:fixity_alg_id]
+      bag_id: params[:bag]&.id,
+      fixity_alg_id: params[:fixity_alg]&.id
     )
     if existing
       render nothing: true, status: 409 and return
@@ -50,9 +50,13 @@ def create
 
   private
 
+  SCALAR_PARAMS = [:value, :created_at]
+  ASSOCIATED_PARAMS = [:bag, :node, :fixity_alg]
 
   def create_params(params)
-    params.permit(MessageDigest.attribute_names)
+    params
+      .permit(SCALAR_PARAMS)
+      .merge(params.slice(*ASSOCIATED_PARAMS))
   end
 
 end

app/controllers/replication_transfers_controller.rb

@@ -16,9 +16,9 @@ class ReplicationTransfersController < ApplicationController
   def index
     @replication_transfers = ReplicationTransfer.updated_after(params[:after])
       .updated_before(params[:before])
-      .with_bag_id(params[:bag_id])
-      .with_to_node_id(params[:to_node_id])
-      .with_from_node_id(params[:from_node_id])
+      .with_bag(params[:bag])
+      .with_to_node(params[:to_node])
+      .with_from_node(params[:from_node])
       .with_store_requested(params[:store_requested])
       .with_stored(params[:stored])
       .with_cancelled(params[:cancelled])
@@ -77,8 +77,22 @@ def destroy
 
 
   private
+
+  SCALAR_PARAMS = [
+    :link, :fixity_nonce, :fixity_value,
+    :created_at, :updated_at,
+    :replication_id, :store_requested, :stored,
+    :cancelled, :cancel_reason, :cancel_reason_detail
+  ]
+  ASSOCIATED_PARAMS = [
+    :bag, :from_node, :to_node, :protocol, :fixity_alg
+  ]
+
+
   def create_params(params)
-    params.permit(ReplicationTransfer.attribute_names)
+    params
+      .permit(SCALAR_PARAMS)
+      .merge(params.slice(*ASSOCIATED_PARAMS))
   end
 
 

app/controllers/restore_transfers_controller.rb

@@ -16,9 +16,9 @@ class RestoreTransfersController < ApplicationController
   def index
     @restore_transfers = RestoreTransfer.updated_after(params[:after])
       .updated_before(params[:before])
-      .with_bag_id(params[:bag_id])
-      .with_to_node_id(params[:to_node_id])
-      .with_from_node_id(params[:from_node_id])
+      .with_bag(params[:bag])
+      .with_to_node(params[:to_node])
+      .with_from_node(params[:from_node])
       .with_accepted(params[:accepted])
       .with_finished(params[:finished])
       .with_cancelled(params[:cancelled])
@@ -74,8 +74,20 @@ def destroy
   end
 
   private
+
+  SCALAR_PARAMS = [
+    :link, :created_at, :updated_at,
+    :restore_id, :accepted, :finished,
+    :cancelled, :cancel_reason, :cancel_reason_detail
+  ]
+  ASSOCIATED_PARAMS = [
+    :bag, :from_node, :to_node, :protocol
+  ]
+
   def create_params(params)
-    params.permit(RestoreTransfer.attribute_names + [:requester])
+    params
+      .permit(SCALAR_PARAMS)
+      .merge(params.slice(*ASSOCIATED_PARAMS))
   end
 
 

app/models/bag.rb

@@ -34,23 +34,24 @@ def self.find_fields
   ### Associations
   belongs_to :ingest_node, :foreign_key => "ingest_node_id", :class_name => "Node",
              autosave: true, inverse_of: :ingest_bags
+  validates_associated :ingest_node
+
   belongs_to :admin_node, :foreign_key => "admin_node_id", :class_name => "Node",
              autosave: true, inverse_of: :admin_bags
+  validates_associated :admin_node
+
   belongs_to :member, :foreign_key => "member_id", :class_name => "Member",
              autosave: true, inverse_of: :bags
-
-  has_many :message_digests, autosave: true, dependent: :destroy, inverse_of: :bag
-  validates_associated :message_digests
-
-  has_many :fixity_checks, inverse_of: :bag
-  has_many :ingests, inverse_of: :bag
+  validates_associated :member
 
   belongs_to :version_family, :inverse_of => :bags, autosave: true
   validates_associated :version_family
 
+  has_many :message_digests, autosave: true, dependent: :destroy, inverse_of: :bag
+  has_many :fixity_checks, inverse_of: :bag
+  has_many :ingests, inverse_of: :bag
   has_many :replication_transfers, autosave: true, inverse_of: :bag
   has_many :restore_transfers, autosave: true, inverse_of: :bag
-
   has_many :bag_nodes, inverse_of: :bag
   has_many :replicating_nodes, through: :bag_nodes, source: :node
 
@@ -80,9 +81,9 @@ def self.find_fields
   ### Scopes
   scope :updated_before, ->(time) { where("updated_at < ?", time) unless time.blank? }
   scope :updated_after, ->(time) { where("updated_at > ?", time) unless time.blank? }
-  scope :with_admin_node_id, ->(id) { where(admin_node_id: id) unless id.blank? }
-  scope :with_ingest_node_id, ->(id) { where(ingest_node_id: id) unless id.blank? }
-  scope :with_member_id, ->(id) { where(member_id: id) unless id.blank? }
+  scope :with_admin_node, ->(node) { where(admin_node: node) unless node.new_record? }
+  scope :with_ingest_node, ->(node) { where(ingest_node: node) unless node.new_record? }
+  scope :with_member, ->(member) { where(member: member) unless member.new_record? }
   scope :with_bag_type, ->(bag_type) { where(type: bag_type) unless bag_type.blank? }
   scope :replicated_by, ->(nodes) {
     unless nodes.empty?
@@ -110,9 +111,18 @@ def self_legal_if_first_version?
 
   def set_attributes_with_associations(new_attributes, &block)
     new_attributes = new_attributes.with_indifferent_access
-    self.attributes = new_attributes.slice(*attribute_names)
-    self.replicating_nodes = new_attributes[:replicating_nodes]
+    self.uuid           = new_attributes[:uuid]
+    self.local_id       = new_attributes[:local_id]
+    self.size           = new_attributes[:size]
+    self.version        = new_attributes[:version]
+    self.type           = new_attributes[:type]
+    self.created_at     = new_attributes[:created_at]
+    self.updated_at     = new_attributes[:updated_at]
+    self.member         = new_attributes[:member]
+    self.ingest_node    = new_attributes[:ingest_node]
+    self.admin_node     = new_attributes[:admin_node]
     self.version_family = new_attributes[:version_family]
+    self.replicating_nodes = new_attributes[:replicating_nodes]
     if self.is_a?(DataBag)
       self.rights_bags = new_attributes[:rights_bags]
       self.interpretive_bags = new_attributes[:interpretive_bags]

app/models/fixity_check.rb

@@ -12,6 +12,8 @@ def self.find_fields
 
   belongs_to :node, inverse_of: :fixity_checks
   belongs_to :bag, inverse_of: :fixity_checks
+  validates_associated :node
+  validates_associated :bag
 
   ### ActiveModel::Dirty Validations
   validates :fixity_check_id, read_only: true, on: :update
@@ -36,8 +38,8 @@ def self.find_fields
   scope :created_after, ->(time) { where("created_at > ?", time) unless time.blank? }
   scope :created_before, ->(time) { where("created_at < ?", time) unless time.blank? }
   scope :with_success, ->(success) { where(success: success) unless success.blank? }
-  scope :with_bag_id, ->(id) { where(bag_id: id) unless id.blank? }
-  scope :with_node_id, ->(id) { where(node_id: id) unless id.blank? }
+  scope :with_bag, ->(bag) { where(bag: bag) unless bag.new_record? }
+  scope :with_node, ->(node) { where(node: node) unless node.new_record? }
   scope :latest_only, ->(flag) do
     unless flag.blank?
       joins("INNER JOIN (

app/models/ingest.rb

@@ -11,10 +11,12 @@ def self.find_fields
   end
 
   belongs_to :bag, inverse_of: :ingests
-
+  validates_associated :bag
+
   has_many :node_ingests, inverse_of: :ingest, dependent: :destroy,
     before_add: :fail_unless_new,
     before_remove: :fail_unless_new
+
   has_many :nodes, through: :node_ingests, source: :node
 
   ### ActiveModel::Dirty Validations
@@ -33,7 +35,7 @@ def self.find_fields
   ### Scopes
   scope :created_after, ->(time) { where("created_at > ?", time) unless time.blank? }
   scope :created_before, ->(time) { where("created_at < ?", time) unless time.blank? }
-  scope :with_bag_id, ->(id) { where(bag_id: id) unless id.blank? }
+  scope :with_bag, ->(bag) { where(bag: bag) unless bag.new_record? }
   scope :with_ingested, ->(ingested) { where(ingested: ingested) if [true,false].include?(ingested) }
   scope :latest_only, ->(flag) do
     unless flag.blank?

app/models/message_digest.rb

@@ -31,7 +31,7 @@ def self.find_fields
   ### Scopes
   scope :created_after, ->(time) { where("created_at > ?", time) unless time.blank? }
   scope :created_before, ->(time) { where("created_at < ?", time) unless time.blank? }
-  scope :with_bag_id, ->(id) { where(bag_id: id) unless id.blank? }
+  scope :with_bag, ->(bag) { where(bag: bag) unless bag.new_record? }
 
 
 end

app/models/replication_transfer.rb

@@ -42,6 +42,11 @@ def cancel!(reason, detail)
   belongs_to :protocol
   has_one :bag_man_request, inverse_of: :replication_transfer
 
+  validates_associated :from_node
+  validates_associated :to_node
+  validates_associated :bag
+  validates_associated :fixity_alg
+  validates_associated :protocol
 
   ### Callbacks
   after_create :add_request_if_needed
@@ -87,9 +92,9 @@ def cancel!(reason, detail)
   ### Scopes
   scope :updated_after, ->(time) { where("updated_at > ?", time) unless time.blank? }
   scope :updated_before, ->(time) { where("updated_at < ?", time) unless time.blank? }
-  scope :with_bag_id, ->(id) { where(bag_id: id) unless id.blank? }
-  scope :with_from_node_id, ->(id) { where(from_node_id: id) unless id.blank? }
-  scope :with_to_node_id, ->(id) { where(to_node_id: id) unless id.blank? }
+  scope :with_bag, ->(bag) { where(bag: bag) unless bag.new_record? }
+  scope :with_from_node, ->(node) { where(from_node: node) unless node.new_record? }
+  scope :with_to_node, ->(node) { where(to_node: node) unless node.new_record? }
   scope :with_store_requested, ->(v){ where(store_requested: v) if [true,false].include?(v) }
   scope :with_stored, ->(v){ where(stored: v) if [true,false].include?(v) }
   scope :with_cancelled, ->(v){ where(cancelled: v) if [true,false].include?(v) }