developer.jelix.org is not used any more and exists only for history. Post new tickets on the Github account.
developer.jelix.org n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.

Opened 13 years ago

Closed 13 years ago

#342 closed bug (fixed)

Pb using jForm and DAO : empty submitted fields vs. int/float/numeric columns

Reported by: tefnout Owned by: laurentj
Priority: normal Milestone: Jelix 1.0 RC1
Component: jelix:forms Version: 1.0 beta 3.1
Severity: major Keywords:
Cc: Blocked By:
Blocking: Documentation needed:
Hosting Provider: Php version: 5.2.4

Description

First of all, we talk here about fields that are not filled by user before submitting a form and using dao.

jForms objects can be filled-up using initFromRequest (jFormBase.class.php l.92). and then saved using saveToDao (l.198 )

if($value === null) $value=''; 

at jFormBase.class.php l.92, and

 $this->_container->datas[$name]= $value; 

l.111 , implies for fields that are not 'uploads' oir 'checkboxes' that their value (again if not filled by user) in the jForm is an empty String.. why not..

Moreover, l.198 in method saveToDao

$daorec->$name = $this->_container->datas[$name]; 

prepares the record to be updated/inserted defining the value to save is still and empty string.

But preparePHPExpr method defined and used by jDaoGenerator for generating update/insert methods contains thoses lines (here for integer columns, but the same kind for double and numeric):

if($checknull){
  $expr= '('.$expr.' === null ? \''.$opnull.'NULL\' : '.$forCondition.'intval('.$expr.'))';
}else{
  $expr= $forCondition.'intval('.$expr.')';
}

so the generated request part is :

COLUMN NAME = ((RECORD->ATTR === null) ? 'NULL' : intval($record->nbconso))

Problem is that, using saveToDao (that calls update/insert) on a jForms initialized with initFromRequest, RECORD->ATTR is never null but valued as an empty string. So, intval(RECORD->ATTR) returns 0, which is saved instead of 'NULL' or having a SQL error.

The generated expression should ensure that, for numeric types (int,float,double,numeric) the given value is not an empty string and otherwise maybe return an error . Another solution is to avoid default value set l.92 of jFormBase.class.php. (more/toomuch intrusive ?)

Change History (3)

comment:1 follow-up: Changed 13 years ago by laurentj

  • Component changed from jelix to jelix:forms
  • Milestone set to Jelix 1.0 RC1
  • Owner set to laurentj
  • Status changed from new to assigned
if($value === null) $value=''; 

In fact, in theory, this test is never true. there is always a value or an empty string, even if the user didn't fill a field, except when the field is a checkbox. In this case, the browser doesn't send a parameter for a unchecked checkbox. And there is a special treatment for checkoxes as you can see in the code.

So there is always at least an empty string, for each field of the form.

An other exception where it can be null : it will appear for a future feature. This feature is the possibility to have a form splitted on several page. So you will have the possibility to have some field on a first page, and other fields on a second page etc.. So, when you will submit one of this page, you won't have all field value at the same time. However this test is bugged for a such feature (that's why there is a @todo comment near it).

So, this is true that the inserted value for a numeric types is always 0 for empty strings, although it should be null for unrequired field. This is bug.

However, I don't see where there is a SQL error... Which sql error do you talk about ? Have you got an example ?

The generated expression should ensure that, for numeric types (int,float,double,numeric) the given value is not an empty string and otherwise maybe return an error

why not...

Another solution is to avoid default value set l.92 of jFormBase.class.php

The remove of this test won't resolve anything as I explained before.

comment:2 in reply to: ↑ 1 Changed 13 years ago by tefnout

Ok, you're really right about $value. Sorry.

I agree there is a bug for NULL-authorized column.

Nevertheless, I'm not sure it's right to consider 0 as a default value for NOT-NULL 'numeric' columns. The only default value should be the one specified in the DB schema or in the correponding DAO property. That's why I awkwardly talked about an "SQL error" because, for me, default values must be explicitly defined. So that, with no default value, a NOT NULL column having a SET to 'nothing ' implies an incorrect SQL query..

An idea is to force user to define "default" for a DAO property for NON-NULL columns (and, by default, take the one defined for the DB column)

comment:3 Changed 13 years ago by laurentj

  • Resolution set to fixed
  • Status changed from assigned to closed

Bug fixed in the trunk: For numerical required property in dao, if a field is empty, jforms set the default value of the dao property in this property.

Note: See TracTickets for help on using tickets.