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

Fixes Column 'dmsf_mail_notification' cannot be null #1567

Merged

Conversation

liaham
Copy link
Contributor

@liaham liaham commented Nov 25, 2024

The partial app/views/dmsf_state/_user_pref.html.erb uses a condition to include the email_notify setting only if the corresponding notified_event is enabled. The database field email_notify, however, does not accept a null value. But this is what occurs when the notified event is disabled.

This patch changes the DmsfStateController to assign params[:email_notify] only if the notified_event for dmsf_legacy_notifications is enabled.

The partial app/views/dmsf_state/_user_pref.html.erb uses a condition
to include the email_notify setting only if the corresponding
notified_event is enabled. The database field email_notify, however,
does not accept a null value. But this is what occurs when the notified
event is disabled.

This patch changes the DmsfStateController to assign params[:email_notify]
only if the notified_event for dmsf_legacy_notifications is enabled.
@picman picman self-requested a review November 25, 2024 09:53
@picman
Copy link
Collaborator

picman commented Nov 25, 2024

Please fix the tests.

@picman picman added this to the 4.0.0 milestone Nov 25, 2024
@picman picman merged commit 5412ab1 into danmunn:devel Nov 25, 2024
3 checks passed
@picman
Copy link
Collaborator

picman commented Nov 25, 2024

Thank you!

Copy link
Collaborator

@picman picman left a comment

Choose a reason for hiding this comment

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

ok

@liaham liaham deleted the bugfix/dmsf_mail_notification_cannot_be_null branch January 21, 2025 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants