"All" method parameter "inactive" seems to be broken

[saboter]

Hello,

according to docs the inactive param set to true should return both active and inactive entity records:

# retrieves all active or inactive employees
  qbo_api.all(:employees, inactive: true).each do |e|
    p "#{e['Id']} #{e['DisplayName']}"
  end

Current default is inactive = false. Which kind of indicates that it will return only active records.
Currently both active and inactive records are returned when the inactive param is set to false (tested on TaxCode). Which looks little bit like a change in default QBO behavior.

def build_all_query(entity, select: nil, inactive: false)
      select ||= "SELECT * FROM #{singular(entity)}"
      select += join_or_start_where_clause!(select: select) + 'Active IN ( true, false )' if inactive
      select
end

If the inactive param should behave as expected it should always add condition e.g.:

def build_all_query(entity, select: nil, inactive: false)
      select ||= "SELECT * FROM #{singular(entity)}"
      active_condition =  inactive ? 'Active IN ( true, false )' : 'Active = true'
      select += join_or_start_where_clause!(select: select) + active_condition
      select
end

Thank you.

[minimul]

Thanks for reporting. Can you submit a fix?

[saboter]

Hi @minimul,

the theoretical fix which always added 'Active = true' when inactive = false is tricky. Not all entities have Active parameter so far I checked in QBO Api docs.

More flexible solution would be to add support for where params into "all" method and remove the inactive param support. You can currently use custom select which allows you to filter (or you can use get). This could be simplified with additional "where:" param support.

  # retrieves all customers by groups of 2 using a custom select query
  where = "WHERE Id IN ('5', '6', '7', '8', '9', '10')"
  qbo_api.all(:customer, max: 2, select: "SELECT * FROM Customer #{where}").each do |c|
    p c['DisplayName']
  end

  # Example usage with non-existing where param for all method

  qbo_api.all(:tax_codes, max: 2, where: "Active = true").each do |c|
     p c['DisplayName']
  end

No time for proper PR with specs on my side currently. Sorry.

[minimul]

Thanks @saboter for the feedback and work-around.

I'll take a deeper look myself when I can.