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

Fix/583 #588

Merged
merged 8 commits into from
Feb 13, 2022
Merged

Fix/583 #588

merged 8 commits into from
Feb 13, 2022

Conversation

Marlysson
Copy link
Contributor

Closes #583

@josephmancuso
Copy link
Member

Can we get these tests fixed? looks like just 1 test is failing

@josephmancuso
Copy link
Member

I was going to work on this issue but if you already fixed it lets just get the tests fixed and ill code review this

@Marlysson
Copy link
Contributor Author

Marlysson commented Feb 8, 2022

Ohh I see, seems the mysql grammar for drop table is wrong 🤔 .. using it with double quotes.
I don't have mysql installed on my pc at moment.

@Marlysson
Copy link
Contributor Author

@josephmancuso It's done.

@josephmancuso
Copy link
Member

@Marlysson i changed the code a bit. You were not hydrating the model as a way to hide the hidden fields.

The problem with that approach is that if you do not hydrate the fields on the model then it will never be accessible when handling the relationships. We need them to be accessible until you serialize it. Then at that point it should be hidden from the serialization

Copy link
Member

@josephmancuso josephmancuso left a comment

Choose a reason for hiding this comment

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

I actually modified the behavior only in the collection class. The tests you wrote are still passing 👍

@josephmancuso josephmancuso merged commit 397564a into MasoniteFramework:2.0 Feb 13, 2022
@Marlysson
Copy link
Contributor Author

Awesome!

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.

Unable to specify certain data to return from an association model
3 participants