-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 belongsTo relationship field name. #4228
Conversation
@emptynick I think it should be better to revert this commit. So it doesn't matter which one we use in name as far as people know that they have to order them in a certain way. What this PR removes is validation for belongsTo that could have been set in the field but now can't because we can't set validation rules for relationships. Other bug #4437 as reference for the problem still happening. |
Revert thedevdojo#4228, fix saving ambiguity in saving belongsTo value
@MrCrayon Could this be the cause of "Not null violation" errors that I see after updating Voyager from 1.2.6 to 1.2.7? To reproduce: I have a table/bread with a belongsTo relationship to another table/bread. When I open an existing record for editing and try to save it (so without making any changes), I'll get a "not null violation" on the belongsTo field. Note that this field had a value when I saved it. When I downgrade to 1.2.6 and try the same steps with the same record, all works fine again. |
@madman-81 quite likely that's the reason. You can update again your platform and change fields order, that should fix it. For more info you can check my PR #4439 and related issues linked there. |
@MrCrayon Yes, that has done the trick! Thanks for the quick reply. Does your new PR fix this problem? In my opinion the order of the columns should not matter for the behaviour. Ideally you should be able to change the whatever_id field to a relationship instead of having them both in the bread configuration. |
@madman-81 yes, my PR makes the order irrilevant. |
* Fix belongsTo not being saved in some configurations Revert #4228, fix saving ambiguity in saving belongsTo value * This line is not needed anymore
This issue was confirmed by 3 users in this issue.
Credits to @tunapiq for suggesting this fix and mine for making the PR.