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

Fixed #12724 - fieldset not saving on model #12726

Merged
merged 1 commit into from
Mar 24, 2023
Merged

Conversation

snipe
Copy link
Owner

@snipe snipe commented Mar 24, 2023

I'm honestly not sure how this ever worked. The field name itself is just wrong. @uberbrady am I missing something here? I know we livewire'd this up, but I'm not sure how this worked in the first place (which I know it did.)

Signed-off-by: snipe <snipe@snipe.net>
@what-the-diff
Copy link

what-the-diff bot commented Mar 24, 2023

PR Summary

  • Renaming of custom_fieldset The custom_fieldset was renamed to fieldset_id for better clarity.
  • Handling fieldset_id in store method If a value is present for fieldset_id, the model's field set id will be assigned accordingly; otherwise, null is assigned.
  • Updating fieldset_id in update method Default values are properly managed when updating fieldset_id, ensuring accuracy and consistency in the models.

Copy link
Collaborator

@uberbrady uberbrady left a comment

Choose a reason for hiding this comment

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

I mean, this seems pretty straightforward? I f it actually, like, works, then I'm absolutely all for it. If it was me who missed this (and, yes, I'm the last person who had hands on this), then I'm happy to take the blame. I kinda understand your reticence, because, like, this should've already been working, and it'd be weird if it had somehow stopped working. But, regardless, seems like a fair set of changes and I will accept them.

@snipe snipe merged commit aa4bdd4 into develop Mar 24, 2023
@snipe snipe deleted the fixes/12724_custom_fields branch March 24, 2023 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants