coldbox-modules / quick

A ColdBox ORM Engine
https://quick.ortusbooks.com
MIT License
23 stars 19 forks source link

Fix a bug when calling isNullValue #213

Closed ryanalbrecht closed 1 year ago

ryanalbrecht commented 1 year ago

By default when calling getInstance('entity') the properties for the given entity will be an actual null value. When retrieving an entity from the database, null fields in the database will be converted to empty strings in the entity.

If you create a new entity and then save it. Any properties that did not have their values set will still be null. This will cause an exception if you then try to retrieve a relationship which has withDefault() set. This is caused by cfparam raising an exception if you return null into the 'default' argument.

E.g.

//if getValue returns null, an exception will be raised
cfparam( name="someKey", default=getValue( ) )
elpete commented 1 year ago

Are these two PR's in one then? I'd also love a test for clone. (Also also, could we just do clone instead of cloneEntity?)

ryanalbrecht commented 1 year ago

@elpete yes this is two pull requests in one. The first was a bug fix from a while ago. Im not two sure how to create two separate pull request using git.

I will implement some tests and rename the function.

ryanalbrecht commented 1 year ago

I added some test and also added coverage for the first pull request's bug fix.

ryanalbrecht commented 1 year ago

@elpete I cleaned a bunch of stuff up and this PR should be ready for review and /or merge

elpete commented 1 year ago

Thank you for the code and especially thank you for your patience.

ryanalbrecht commented 1 year ago

Thanks elpete, sorry for the messy PRs.

ryanalbrecht commented 1 year ago

@elpete quick question. Refering to 'formatting and cleanup' commit you made on this PR.

Did you manually make those changes or did you use an automated tool?

elpete commented 1 year ago

Ha, the PR wasn't messy. I ran it through CFFormat and changed a couple of the TestBox assertions from expect( a eq b ).toBeTrue() to expect( a ).toBe( b ).