PDA

View Full Version : [FIXED-532][3.1] Problem loading values into store



mikecar
11 Jan 2010, 6:44 PM
I believe I found a rather serious bug in Ext 3.1.0. It concerns data stores,
and how they are updated from server.

Creating a runnable example is quite difficult, as you would need server code
as well. OTOH, explaining the issue is pretty simple, and, after a couple of
hours of debugging, I can pinpoint the source line(s), and suggest a fix.

The use case is as follows:

assume that you have a server database table, that contains two fields,
A & B. The value of B is dependent on the value of A (in my case,
the user enters A, and the server calculates B using a lookup).
Example : A is a customer code, and B is the customer description.

Now, assume that you initially have an empty data store.
The user creates a new record, and adds a value for A (B is read-only).
The server will lookup the value for B, and return an updated record,
with a real ID, and B filled-in.
Next, the user changes the value for A, to a value that has no
corresponding value for B (lookup fails). After a store.save(),
the server will return an updated record, with B blank.
However, due to the bug I am describing, the user will still see
field B having is old value.

The offending code is file src/data/dataReader.js, in two places:



1. function realize line 117 :

rs.fields.each(function(f) {
if (data[f.name] !== f.defaultValue) {
rs.data[f.name] = data[f.name];

2. function update, line 153:

rs.fields.each(function(f) {
if (data[f.name] !== f.defaultValue) {
rs.data[f.name] = data[f.name];
assuming that fields have been set up with empty defaultValues,
this means that if the incoming value from the server is empty,
(and therefore equal to the default value)
it will NOT be set in data[], which will continue to contain the
original value.

The above scenario is what actually happened in my application.
you could get same results with a single field that is initially
entered by the user, and subsequently cleared to an empty value
by code. the user would not see the change, and the wrong value
would be submitted to the server on the next save.

I cannot imagine what the person who wrote this code had in mind.
After all, javascript AFAIK does not have setter functions, so the
assignment statement will not have higher performance cost than the
equality test, neither any side-effects (event triggers etc).
So, why on earth is this test needed ?

The assumption behind that code only works at the
FIRST load of a datastore, not subsequent updates.

I verified locally that, if you remove the equality test, all works as
expected. Perhaps there is another reason for this test, and some other
part of the code will break, but I seriously doubt it.

I classify this as a very serious bug, because it is not visual only,
it also corrupts data, which IMHO is the single thing a framework is not
allowed to do. what if the field was not visible, and the user did not
spot the problem ?

evant
1 Feb 2010, 12:58 AM
I'm not really sure what the logic behind having the defaultValue check is, it was the most recent change to the class, but it doesn't really explain why it happened.

I don't see why it can't be removed. Any objectors?

mikecar
4 Feb 2010, 2:18 AM
Evant, I dont see any takers, so what do you think ?

rich02818
4 Feb 2010, 6:29 AM
Perhaps you can ask whomever made the change what they were thinking....

Jamie Avins
23 Feb 2010, 11:16 AM
Reverted in SVN 6142.