patternrubyrailsMinor

Searching purchase records, with or without a vendor criterion

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

withoutsearchingwithrecordsvendorpurchasecriterion

Problem

I'm trying to figure out a better way to have one query here. I want to be able to send something to last where statement a wildcard so I can select all vendors. Right now if I don't include that line it doesn't filter by vendor so I essentially get all the purchase requests.

Any thoughts of a cleaner way to do these queries?

if @vendor == "0" #checks for vendor
    @purchase_requests = PurchaseRequest.includes(:purchase_order)
                          .where(:created_at => @date_start..@date_end)
                          .where(:total_cost => @cost_beginning..@cost_end)
  else
    @purchase_requests = PurchaseRequest.includes(:purchase_order)
                          .where(:created_at => @date_start..@date_end)
                          .where(:total_cost => @cost_beginning..@cost_end)
                          .where("purchaseorder.VendorRef_ListID = ?", @vendor)
  end

Solution

The simplest refactor would be:

# this is the base query
requests = PurchaseRequest.includes(:purchase_order)
             .where(:created_at => @date_start..@date_end)
             .where(:total_cost => @cost_beginning..@cost_end)

# then we add additional conditions if required 
requests =
  unless @vendor == "0" # if vendor, get specific purchase request
    request.where("purchaseorder.VendorRef_ListID = ?", @vendor)
  end

If you find that you do certain queries often, the it's probably a chance to use scopes. You may already know about scopes, but if not, check out Mastering ActiveRecord and Arel.

class PurchaseRequest (date_range) {
    where(:created_at => date_range)
  }

  # requests with a certain cost range
  scope :cost_between, ->(date_range) {
    where(:total_cost => date_range)
  }

  # find request for a specific vendor
  scope :for_vendor, ->(vendor) {
    # this scope does an inner join to purchase_order and adds
    # a condition that the PurchaseOrder must belong to a specific
    # vendor.  That is, only requests matching the vendor are returned
    joins(:purchase_order).merge( PurchaseOrder.for_vendor(vendor) )
  }
end

class PurchaseOrder (vendor) {
    where(:VendorRef_ListID => vendor)
  }
end

So with scopes added, you can now refactor your code like so:

# this is the base query
requests = PurchaseRequest.includes(:purchase_order)
             .date_between(@date_start..@date_end)
             .cost_between(@cost_beginning..@cost_end)

# if vendor, get specific request
requests = request.for_vendor(@vendor) unless @vendor == "0"

Code Snippets

# this is the base query
requests = PurchaseRequest.includes(:purchase_order)
             .where(:created_at => @date_start..@date_end)
             .where(:total_cost => @cost_beginning..@cost_end)

# then we add additional conditions if required 
requests =
  unless @vendor == "0" # if vendor, get specific purchase request
    request.where("purchaseorder.VendorRef_ListID = ?", @vendor)
  end

class PurchaseRequest < ActiveRecord::Base
  has_one :purchase_order

  # requests within a certain data
  scope :date_between, ->(date_range) {
    where(:created_at => date_range)
  }

  # requests with a certain cost range
  scope :cost_between, ->(date_range) {
    where(:total_cost => date_range)
  }

  # find request for a specific vendor
  scope :for_vendor, ->(vendor) {
    # this scope does an inner join to purchase_order and adds
    # a condition that the PurchaseOrder must belong to a specific
    # vendor.  That is, only requests matching the vendor are returned
    joins(:purchase_order).merge( PurchaseOrder.for_vendor(vendor) )
  }
end

class PurchaseOrder < ActiveRecord::Base
  # purchase orders belonging to a specific vendor
  scope :for_vendor, ->(vendor) {
    where(:VendorRef_ListID => vendor)
  }
end

# this is the base query
requests = PurchaseRequest.includes(:purchase_order)
             .date_between(@date_start..@date_end)
             .cost_between(@cost_beginning..@cost_end)

# if vendor, get specific request
requests = request.for_vendor(@vendor) unless @vendor == "0"

Context

StackExchange Code Review Q#99019, answer score: 2

Revisions (0)

No revisions yet.