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

Try to fix the default values issue when loading records from the database in partial bean mode #755

Merged
merged 1 commit into from Nov 16, 2019

Conversation

@AlexanderYIXIAO
Copy link
Contributor

AlexanderYIXIAO commented Nov 14, 2019

R::load('type', $id) and R::dispense both are based on the load function in class Repository(Fluid/Frozen). It will call the dispense function at first and load some default values. When you load the default values, it will change the __info['changelist']. It is OK when just dispense a new bean but not OK when loading a record from the database especially you want to use partial bean mode later.

#754

R::load('type', $id) and R::dispense both are based on the load function in class Repository(Fluid/Frozen). It will call the dispense function at first and load some default values. When you load the default values, it will change the __info['changelist']. It is OK when just dispense a new bean but not OK when load a record from the database especially you want to use partial bean mode later.
@gabordemooij

This comment has been minimized.

Copy link
Owner

gabordemooij commented Nov 14, 2019

Thanks for the PR.
Some considerations:

  1. Do we need a unit test for this fix - to make sure there will be no regressions in the future?
  2. Do we need a feature flag to maintain backward compatibility, maybe people have based their logic on this behaviour?
    I can help with these issues.
@marios88

This comment has been minimized.

Copy link
Contributor

marios88 commented Nov 15, 2019

If i understand correctly, this changes the default behaviour of hasChanged and isTainted when when loading existing beans. dispense behaviour remains the same.

@Lynesth

This comment has been minimized.

Copy link
Contributor

Lynesth commented Nov 15, 2019

Current behavior of RedBean when loading/finding a bean is:

  1. Dispense new bean
  2. Set 'changed' to False and 'changelist' to array()
  3. Call the dispense event
  4. Load data from the database
  5. Fill beans properties with data and set changed to False (Location of this PR)
  6. Call the open event

So currently, when changes are made to the properties of a bean during the dispense event (step 3 of the list above), they are saved in the changelist. They are then completely overwritten during step 5, but the changelist isn't updated and keeps changes that don't exist anymore.

That's the only thing this fixes so it should be more than fine to merge.

@gabordemooij gabordemooij merged commit 897b9ba into gabordemooij:master Nov 16, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.