Skip to content

LG-13345 | FormObject fix in AgreementController#10652

Merged
n1zyy merged 3 commits intomainfrom
mattw/LG-13345-form-object-agreement-controller
May 20, 2024
Merged

LG-13345 | FormObject fix in AgreementController#10652
n1zyy merged 3 commits intomainfrom
mattw/LG-13345-form-object-agreement-controller

Conversation

@n1zyy
Copy link
Contributor

@n1zyy n1zyy commented May 17, 2024

This makes some small adjustments to how bring our FormObject usage in line with best practices.

This makes some small adjustments to how bring our FormObject
usage in line with best practices.
it 'renders the form again' do
put :update, params: params
expect(response).to redirect_to(idv_agreement_url)
expect(response).to render_template('idv/agreement/show')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For reasons I don't fully understand yet, response.body is an empty string here, so I'm only able to make this assertion, but not that it contains the error string that wasn't previously being displayed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Controller specs don't render views by default, which may be causing what you're seeing. You can opt-in if you wanted to specifically assert against view content, though typically that'd happen in the view specs instead.

https://rspec.info/features/6-0/rspec-rails/controller-specs/render-views/

I think what you have here makes sense.

n1zyy added 2 commits May 20, 2024 10:50
changelog: Internal, FormObject, Fix usage in AgreementController
@n1zyy n1zyy marked this pull request as ready for review May 20, 2024 16:15
@n1zyy n1zyy requested a review from a team May 20, 2024 16:15
@n1zyy n1zyy merged commit 5cb708a into main May 20, 2024
@n1zyy n1zyy deleted the mattw/LG-13345-form-object-agreement-controller branch May 20, 2024 17:33
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.

3 participants