View Issue Details

Jump to Notes
IDProjectCategoryView StatusDate SubmittedLast Update
0003148OXID eShop (all versions)1.01. Products (product, categories, manufacturer, promotions etc.)public2011-08-16 14:292012-12-07 14:28
Reporterd3 Assigned To 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version4.5.1 revision 38045 
Fixed in Version4.5.2 revision 38481 
Summary0003148: save-function causes fatal error
Descriptionoxarticle save-function causes fatal error.

V4.5.0:
Fatal error: Call to a member function setValue() on a non-object in /this/is/shop-root/core/oxarticle.php on line 1951
Steps To Reproduce    public function save()
    {
        $myConfig = $this->getConfig();

        $this->oxarticles__oxthumb = new oxField(basename($this->oxarticles__oxthumb->value), oxField::T_RAW);
        $this->oxarticles__oxicon = new oxField(basename($this->oxarticles__oxicon->value), oxField::T_RAW);
        $iPicCount = $myConfig->getConfigParam( 'iPicCount');
        for ($i=1; $i <= $iPicCount; $i++) {
            if ( isset($this->{'oxarticles__oxpic'.$i}) ) {
                $this->{'oxarticles__oxpic'.$i}->setValue(basename($this->{'oxarticles__oxpic'.$i}->value));
            }
        }

        if ( ( $blRet = parent::save() ) ) {

            // saving long descrition
            $this->_saveArtLongDesc();
        }

        return $blRet;
    }
Additional Informationshould be:
    public function save()
    {
        $myConfig = $this->getConfig();

        $this->oxarticles__oxthumb = new oxField(basename($this->oxarticles__oxthumb->value), oxField::T_RAW);
        $this->oxarticles__oxicon = new oxField(basename($this->oxarticles__oxicon->value), oxField::T_RAW);
        $iPicCount = $myConfig->getConfigParam( 'iPicCount');
        for ($i=1; $i <= $iPicCount; $i++) {
            if ( $this->getFieldData("oxpic$i") ) {
                 $sFieldName = $this->getFieldLongName("oxpic$i");
                $this->$sFieldName = oxNew("oxField", basename($this->getFieldData("oxpic$i")));
            }
        }

        if ( ( $blRet = parent::save() ) ) {

            // saving long descrition
            $this->_saveArtLongDesc();
        }

        return $blRet;
    }
TagsProducts
ThemeBoth
BrowserAll
PHP Versionany
Database Versionany

Activities

d3

2011-08-16 17:43

reporter   ~0005032

function getFieldLongName (_getFieldLongName) is protected
should be public

svetlana

2011-08-17 16:22

reporter   ~0005047

@developers please investigate it

d3

2011-08-18 07:29

reporter   ~0005052

function setFieldData (_setFieldData) is protected too
should also be public

arvydas_vapsva

2011-08-22 16:31

reporter   ~0005085

correct fix:

    for ( $i = 1; $i <= $iPicCount; $i++ ) {
        $sFieldName = 'oxarticles__oxpic' . $i;
        if ( isset( $this->$sFieldName ) ) {
            $this->_setFieldData( $sFieldName, basename( $this->$sFieldName->value ), oxField::T_RAW );
        }
    }

and why do you need these getters/setters to be public?

tomas_liubinas

2011-08-23 14:32

reporter   ~0005090

oxBase::_getFieldLongName() and especially oxBase::_setFieldData() are not the part of public class interface and are not intended to be called from outside, therefore they are declared as protected. You should use public interface methods (eg. oxBase::assign() or oxField::setValue()) in order to init field values.

d3

2011-08-23 16:02

reporter   ~0005091

@ tomas_liubinas
Because there is no documentation, tutorial or best practice guideline for the correct usage of the api.
Most of the usage is copied from existing oxid code.
Just search for 'new oxField' in the shop souces... you will find many examples of inconsequent api usage (oxNew, oxBase::assign() etc..)

Agreed, oxBase::assign() is the correct method for those purposes.

One question:
in the following line of your fix:
"$this->_setFieldData( $sFieldName, basename( $this->$sFieldName->value ), oxField::T_RAW );"

why the usage of "$this->$sFieldName->value" and not "$this->getFieldData($sFieldName)" ?
Also the usage of oxBase::assign() would be consequent.

arvydas_vapsva

2011-08-26 09:53

reporter   ~0005121

"why the usage of "$this->$sFieldName->value" and not "$this->getFieldData($sFieldName)" ?"

in this case just only due to performace reasons (and because this code is now deprecated), as field name is known (also checked by isset()), so there is no need to call getter. But third party module writers should use oxArticle::_setFieldData()/oxArticle::getFieldData()/oxArticle::_getFieldLongName()