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

Added a concept of default values to forms. Also includes using models & #5634

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

powersurge360
Copy link
Contributor

@powersurge360 powersurge360 commented Feb 24, 2025

Link to pivotal/JIRA issue

  • No JIRA ticket

Is PM acceptance required? (delete one)

  • No - merge after code review approval

Reminder: merge main into this branch and get green tests before merging to main

What was done?

When you use a form's set_attributes_for on fields with defaults and don't set a value, you can get database level failures. This provides two possible fixes; either:
a. Specifically provide a defaults dictionary or
b. Use a model rather than a symbol for the first argument to set_attributes_for

When doing the latter, we inspect the column_defaults on the model and populate the default values with which we initialize the form, so they will never be nil without being explicitly set to nil.

This behavior is opt in and uses new arguments and reflection on the model. It should change no other behavior.

This resolves issues like the ones seen on update_13614c_form_page1.

It will also set us up well for a default QuestionsForm#save method; if all keys are models, then we can assume that their respective attributes are available and we can save it out.

How to test?

  • It's pretty high touch. Doing basically anything in the application will either succeed or fail.
  • There's a chance that this will need to be touched up a little more when it gets first used, I didn't really want to explicitly change any forms, but I look forward to playing with it in the future.

Screenshots (for visual changes)

  • Before
  • After

@powersurge360 powersurge360 added the wip denotes a work in progress that isn't ready for formal review label Feb 24, 2025
Copy link

Heroku app: https://gyr-review-app-5634-7d97a9a4b0fe.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5634 (optionally add --tail)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wip denotes a work in progress that isn't ready for formal review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant