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

Add a failing test and fix the field assigned on set #35

Merged
merged 1 commit into from
Oct 15, 2016
Merged

Add a failing test and fix the field assigned on set #35

merged 1 commit into from
Oct 15, 2016

Conversation

harikt
Copy link
Contributor

@harikt harikt commented Oct 15, 2016

No description provided.

'zim' => 'dim'
]);

$actual = $this->record->getArrayCopy();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The getArrayCopy is failing.

Didn't understand why so. May be late night :) .

There was 1 error:

1) Atlas\Orm\Mapper\RecordTest::testSet
Error: Call to a member function getArrayCopy() on string

/var/www/projects/github.com/harikt/Atlas.Orm/src/Mapper/Related.php:147
/var/www/projects/github.com/harikt/Atlas.Orm/src/Mapper/Record.php:211
/var/www/projects/github.com/harikt/Atlas.Orm/tests/Mapper/RecordTest.php:129

@pmjones
Copy link
Contributor

pmjones commented Oct 15, 2016

Well that's unexpected. I'll take a look.

@pmjones pmjones merged commit 62b2a90 into atlasphp:1.x Oct 15, 2016
@pmjones
Copy link
Contributor

pmjones commented Oct 15, 2016

Aha! It's because Related should only allow you to set null, false, empty array, Record, or RecordSet. Alternatively, getArrayCopy() should check if the thing being copied is a Record or RecordSet before descending into it.

@harikt
Copy link
Contributor Author

harikt commented Oct 15, 2016

I was just writing.

@harikt harikt deleted the related branch October 15, 2016 19:11
@harikt
Copy link
Contributor Author

harikt commented Oct 15, 2016

Need a PR or you patched ?

@harikt
Copy link
Contributor Author

harikt commented Oct 15, 2016

In case , here is a patch #37 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants