Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Mismatch of capitalization between model properties and column names in generated model files #1463

Closed
Nyde opened this issue Jun 3, 2020 · 2 comments · Fixed by #1498

Comments

@Nyde
Copy link

Nyde commented Jun 3, 2020

Please see the discussion on the forum (https://forum.phalcon.io/discussion/20661/modelsave-saves-not-the-entire-model-return-xy-is-required) and the accepted answer (https://forum.phalcon.io/discussion/20661/modelsave-saves-not-the-entire-model-return-xy-is-required#C63301).

Actual Behavior

When you create a model from a table that has capitalized columns, the property is saved in lower case. However, the column map exspects the field name with a leading capital letter, therefore yields "xy is required" on save().

See this table definition:

CREATE TABLE `barcodes` (
  `cid` int(11) NOT NULL AUTO_INCREMENT,
  `aid` int(11) NOT NULL,
  `Barcode` varchar(20) COLLATE utf8_bin NOT NULL,
  PRIMARY KEY (`cid`),
  KEY `articleFK_idx` (`aid`),
  CONSTRAINT `articleFK` FOREIGN KEY (`aid`) REFERENCES `articles` (`aid`) ON DELETE NO ACTION ON UPDATE NO ACTION
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin;

Note the capitalized "B" of "Barcode".

The model generated by webtools.php:

class barcodes extends \Phalcon\Mvc\Model
{
    protected $cid;
    protected $aid;
    protected $barcode;

    public function setCid($cid)
    {
        $this->cid = $cid;

        return $this;
    }

    public function setAid($aid)
    {
        $this->aid = $aid;

        return $this;
    }

    public function setBarcode($barcode)
    {
        $this->barcode = $barcode;

        return $this;
    }

    public function getCid()
    {
        return $this->cid;
    }

    public function getAid()
    {
        return $this->aid;
    }

    public function getBarcode()
    {
        return $this->barcode;
    }

    public function initialize()
    {
        $this->setSource("barcodes");
        $this->belongsTo('aid', '\articles', 'aid', ['alias' => 'articles']);
    }

    public static function find($parameters = null): \Phalcon\Mvc\Model\ResultsetInterface
    {
        return parent::find($parameters);
    }

    public static function findFirst($parameters = null)
    {
        return parent::findFirst($parameters);
    }

    public function columnMap()
    {
        return [
            'cid' => 'cid',
            'aid' => 'aid',
            'Barcode' => 'Barcode' // leading B is capitalized, but the corresponding field is all lower case
        ];
    }

}

When trying to save(), this yields "Barcode is required", even if set. Internally, $this->barcode gets set by setBarcode('somestring');, but the column map generated by webtools.php exspects the field to be named $this->Barcode.

Exspected behaviour

webtools.php should match the keys from the column map with the actual property names, with correct capitalization taken into account.

Details

  • System info and versions (if possible):
  • Phalcon Framework version: 4.0.6
  • PHP Version: 7.4.4
  • Operating System: Windows 10
  • Server: Apache
  • Other related info (Database, table schema): see above
@Jeckerson Jeckerson self-assigned this Jun 3, 2020
@Jeckerson Jeckerson added this to the 4.0.x milestone Jun 3, 2020
@BeMySlaveDarlin
Copy link
Contributor

This is NFR.
Wont be done, because of camelize behavior, which is needed for fields named like group_id.
Changing template behavior to make Barcode be consistent breaks snake_cased fields name mutation.
As said in forums thread, ill recommend to rename fields such as Barcode to barcode or manually change columnMap() method to fulfill that needs.

@Jeckerson Jeckerson added this to Backlog in Phalcon Roadmap Mar 22, 2021
@Jeckerson Jeckerson modified the milestones: 4.0.x, 5.0.x Mar 22, 2021
@BeMySlaveDarlin
Copy link
Contributor

Somehow fixed in #1498

@Jeckerson Jeckerson modified the milestones: 5.0.x, 4.0.x Mar 22, 2021
@Jeckerson Jeckerson moved this from Backlog to Working on it in Phalcon Roadmap Mar 22, 2021
@Jeckerson Jeckerson linked a pull request Mar 22, 2021 that will close this issue
3 tasks
Phalcon Roadmap automation moved this from Working on it to Implemented Mar 22, 2021
@Jeckerson Jeckerson mentioned this issue Mar 22, 2021
@niden niden moved this from Implemented to Released in Phalcon Roadmap Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Phalcon Roadmap
  
Released
Development

Successfully merging a pull request may close this issue.

3 participants