Possible bug found?

2 views
Skip to first unread message

Sergio Viudes

unread,
Feb 17, 2010, 11:48:10 AM2/17/10
to JazzRecord
Hello. First of all sorry about my possible bad english.

I think there is a bug in JazzRecord.Record.prototype.save function.
Let's see.

I wrote a model, like this:

TicketLinea = new JazzRecord.Model({
table: "tickets_lin",
columns: {
producto_id: "number",
precio: "float",
}
});

JazzRecord.migrate();

Well, then I used it like this:

var ticket = TicketLinea.create({
producto_id: 3,
precio: 2
});

ticket.save();
ticket.precio = 4;
ticket.save();
ticket.precio = 5;
ticket.save();

Well, first and second saves are ok, but it crashes on third one.
Why?, because ticket.id is null and it's trying to do "UPDATE tickets
SET producto_id = 3, precio = 2 WHERE id = undefined", ¿¿undefined??
I spent some time reading your code and I found this in
JazzRecord.Record.prototype.save function:

At line 114 we can see:

var data = this.getData();
if(this.isChanged()) {
this.onSave();
data.originalData = this.originalData ? this.originalData : {id:
this.id};
this.options.model.save(data);
this.reload();
// overwrite original data so it is no longer "dirty"
JazzRecord.each(this.options.columns, function(colType, colName)
{
this.originalData[colName] = this[colName];
}, this);
}
else if(!this.id) {
this.onSave();
this.id = this.options.model.save(data);
}

Well, on first save, record isn't changed, so enter to "else" block,
saves record for first time and get a new id.

On second save record is changed and enter to "if" block,
this.originalData is undefined so data.originalData will be {id:
this.id}; then each column value is saved to "this.originalData" (id
column is not saved because "id" is not defined in model as a column).

But the first time we call to "save", record is changed again, an
enter to "if block" again. Now this.originalData is defined (was
defined on previous save), so data.originalData now is equal to
this.originalData. The "data" object is used as parameter to
this.options.model.save(), and this method uses data.originalData.id
to build query string, BUT data.originalData.id WAS NEVER ASSIGNED, so
is undefined and query fails.

I changed previous code to next one and now it works:

var data = this.getData();
if(this.isChanged()) {
this.onSave();
data.originalData = this.originalData ? this.originalData : {id:
this.id};
this.options.model.save(data);
this.reload();
// overwrite original data so it is no longer "dirty"
JazzRecord.each(this.options.columns, function(colType, colName)
{
this.originalData[colName] = this[colName];

this.originalData.id = this.id; // ¡¡¡¡¡ LINE ADDED !!!!!

}, this);
}
else if(!this.id) {
this.onSave();
this.id = this.options.model.save(data);
}

Is this Ok?

Thanks!

Sergio Viudes

unread,
Feb 17, 2010, 12:22:51 PM2/17/10
to JazzRecord
My modification it's not enough to get jazzrecord working ok. So I
added "id" column to all my models and now it's working fine. But I
still think that's not the best solution, right?

Nick Carter

unread,
Feb 18, 2010, 2:04:14 PM2/18/10
to JazzRecord
Hi, Sergio. Thanks for reporting this. Will look into it as soon as
possible.

Nick Carter

unread,
Feb 22, 2010, 7:22:52 AM2/22/10
to JazzRecord
This is now fixed on the latest master build as well as 0.8 branch on
Github, and a test has been put in place to be sure it doesn't happen
again.

Please note that an "id" column will be created automatically if you
do not provide it yourself in the list of columns, and its data
managed appropriately as an integer.

On Feb 17, 12:22 pm, Sergio Viudes <djpep...@gmail.com> wrote:

Sergio Viudes

unread,
Feb 22, 2010, 8:31:00 AM2/22/10
to jazzr...@googlegroups.com
Thanks!, great work!

2010/2/22 Nick Carter <thyn...@gmail.com>
--
You received this message because you are subscribed to the Google Groups "JazzRecord" group.
To post to this group, send email to jazzr...@googlegroups.com.
To unsubscribe from this group, send email to jazzrecord+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/jazzrecord?hl=en.


Reply all
Reply to author
Forward
0 new messages