Would someone like to tell me why this code will not solve m


#1

I am building an invoicing system but cannot use the auto_increment
field to determine the invoice number (because they are running 3
different companies off the one system. I need to find the last
invoice number from any given company and then add 1 to it to get the
next invoice number. BUT, there is a unique case on the very first
invoice produced because there is no earlier invoice number. My
solution is as follows (and does not work)

This is in the ‘create’ controller

 if Invoice.find(:all)
   @temp = Invoice.find_by_sql("select max(inv_number) as inv_number
                           from invoices where company_id = params

[:company_id")
@invoice.inv_number = @temp[0].inv_number +1
else
@invoice.inv_number=1
end

The error I am getting is

NoMethodError in Invoices#create

You have a nil object when you didn’t expect it!
You might have expected an instance of Array.
The error occured while evaluating nil.+


#2

bruce balmer wrote:

if Invoice.find(:all)
@temp = Invoice.find_by_sql(“select max(inv_number) as inv_number
from invoices where company_id
= params[:company_id”)

This line might be part of the problem. the “params[:company_id]” is
being inserted literally into your sql string. You need to do something
like

… = #{params[:company_id]}" to insert it.

WARNING… this is highly unsafe. A user could manually pass all sorts
of badness through the company_id param and totally hose you. Please
find some other (secure) way to do this. The [‘company_id = ?’,
params[:company_id]] approach is better, but I’m not sure how to do this
with the find_by_sql function.

By the way, you could do something like…

Invoice.find(:first, :condition=>[“company_id = ?”,
params[:company_id]], :order=>“inv_number DESC”)

This should give you the record with the highest invoice number.

_Kevin


#3

On Mon, Jan 02, 2006 at 09:10:47PM -0700, Bruce B. wrote:

This is in the ‘create’ controller

Invoice.find(:all) will always be boolean true. If there are no
Invoices,
then find(:all) will return an empty array ([]), which is always boolean
true.

A way of expressing your intentions is:

unless Invoice.count.zero?

   if Invoice.find(:all) 

params[:company_id] is not being interpolated. It’s being passed along
as a
literal value. String interpolation in Ruby is done with #{}. For
example:

foo = ‘Marcel’
“My name is #{foo}”
=> “My name is Marcel”

The following find_by_sql, makes you vulnerable to SQL injection. Rather
than
using find_by_sql, you could use find, passing the :select and the
:conditions options. Something like:

Invoice.find(:first, :select => ‘MAX(inv_number) as inv_number’,
:conditions => [‘company_id = ?’, params[:company_id]).inv_number.to_i

But there is no need to do this MAX business. You can instead use
ActiveRecord’s increment or increment! or increment_counter method:

http://api.rubyonrails.com/classes/ActiveRecord/Base.html#M000768
http://api.rubyonrails.com/classes/ActiveRecord/Base.html#M000727

Increment allows you to avoid the entire conditional.

     @temp = Invoice.find_by_sql("select max(inv_number) as inv_number
                             from invoices where company_id =

params[:company_id")
@invoice.inv_number = @temp[0].inv_number +1
else
@invoice.inv_number=1
end

marcel


#4

Marcel:

Thanks very much. That was awesome!

bruce


#5

On 3.1.2006, at 6.55, Marcel Molina Jr. wrote:

But there is no need to do this MAX business. You can instead use
ActiveRecord’s increment or increment! or increment_counter method:

http://api.rubyonrails.com/classes/ActiveRecord/Base.html#M000768
http://api.rubyonrails.com/classes/ActiveRecord/Base.html#M000727

Increment allows you to avoid the entire conditional.

Also keep in mind that selecting max() from a table is NOT a
transaction safe method. If you have two users inserting an invoice
concurrently, they might end up with the same numbe. That’s why many
database management systems have sequences [1].

//jarkko

[1] http://www.postgresql.org/docs/8.1/interactive/sql-
createsequence.html