search

propelorm / Propel2

Propel2 is an open-source high-performance Object-Relational Mapping (ORM) for modern PHP
http://propelorm.org/
MIT License
1.26k stars 398 forks source link

Saving object with modified primary key #1896

Closed basteyy closed 11 months ago

basteyy commented 2 years ago

This fix put the primary keys into an array while building the object. From this array, the values will be used to create the criteria for updating. After updating an object, the values in the array will be regenerated to ensure multiple changes in one request.

Fixes https://github.com/propelorm/Propel2/issues/978

basteyy commented 2 years ago

The checks are failing because of composer: https://github.com/propelorm/Propel2/runs/7615255384?check_suite_focus=true

dereuromark commented 2 years ago

Why are those checks failing Didnt you use latest master as reference?

basteyy commented 2 years ago

Why are those checks failing Didnt you use latest master as reference?

I used latest master. They failed because:

Error: dealerdirect/phpcodesniffer-composer-installer contains a Composer plugin which is blocked by your allow-plugins config. You may add it to the list if you consider it safe.
You can run "composer config --no-plugins allow-plugins.dealerdirect/phpcodesniffer-composer-installer [true|false]" to enable it (true) or disable it explicitly and suppress this exception (false)
See https://getcomposer.org/allow-plugins

In PluginManager.php line 762:

  dealerdirect/phpcodesniffer-composer-installer contains a Composer plugin w  
  hich is blocked by your allow-plugins config. You may add it to the list if  
   you consider it safe.                                                       
  You can run "composer config --no-plugins allow-plugins.dealerdirect/phpcod  
  esniffer-composer-installer [true|false]" to enable it (true) or disable it  
   explicitly and suppress this exception (false)                              
  See https://getcomposer.org/allow-plugins

See https://github.com/propelorm/Propel2/runs/7263266989?check_suite_focus=true

All other current PR also failing.

Propel requires "spryker/code-sniffer": "^0.17.2", which seems to be the reason for that: https://github.com/spryker/code-sniffer/blob/master/composer.json#L67

dereuromark commented 2 years ago

Yeah, but we have the same here whitelisted So this seems to be composer bug/regression I opened a ticket for now: https://github.com/composer/composer/issues/10981

dereuromark commented 2 years ago

Master is fixed, can you please rebase?

dereuromark commented 2 years ago

I wonder if this should come with a test case cc @mringler

mringler commented 2 years ago

I wonder if this should come with a test case

Yes, @basteyy, could you please add a test verifying the updated behavior?

If you want to run test on your machine, there is a page with hints how to get it running. It is partially outdated though, specifically the part about "custom credentials". Not sure if that ever worked, but on MySQL, AFAIK you need to use user "admin" and password "" (empty string). Might be easier to use SQLite. Anyway, let me know if you want help with that.

dereuromark commented 2 years ago

Any update on this one?

basteyy commented 2 years ago

Any update on this one?

I will update it next week

dereuromark commented 2 years ago

ping @basteyy

basteyy commented 2 years ago

ping @basteyy

Setting up the local test environment needed time, and I'm struggling with a test case. Thanks for pinging, try to modify the patch end of this week.

dereuromark commented 1 year ago

Testing should now be easier, see updated docs

dereuromark commented 1 year ago

Please note: We are soon releasing new beta, do you plan on fitting this into the release?