FI-2318 Allow for form data at session create endpoint #429
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #429 +/- ##
==========================================
+ Coverage 77.02% 77.08% +0.05%
==========================================
Files 218 220 +2
Lines 10914 10946 +32
Branches 1026 1029 +3
==========================================
+ Hits 8407 8438 +31
- Misses 1928 1929 +1
Partials 579 579
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
* Add Data Rights Legend * Remove copyright from README
| @@ -10,7 +10,9 @@ class Create < Controller | |||
|
|
|||
| def handle(req, res) | |||
| params = req.params.to_h | |||
| params.merge!(JSON.parse(req.body.string).symbolize_keys) unless req.body.string.blank? | |||
| unless req.body.string.blank? || req.env['CONTENT_TYPE'].include?('multipart/form-data') | |||
There was a problem hiding this comment.
Compound conditions with unless are really confusing, so you should change this to use if.
* bump version * update changelog
| @@ -10,7 +10,9 @@ class Create < Controller | |||
|
|
|||
| def handle(req, res) | |||
| params = req.params.to_h | |||
| params.merge!(JSON.parse(req.body.string).symbolize_keys) unless req.body.string.blank? | |||
| if !req.body.string.blank? && !req.env['CONTENT_TYPE'].include?('multipart/form-data') | |||
There was a problem hiding this comment.
Use present? instead of !blank?.
Why check that the content type isn't form data instead of checking whether the content type is json? Also, that isn't the only content type used by forms.
There was a problem hiding this comment.
Sounds good, I've changed it to just check for json instead before attempting to parse as a json. I'm still learning in's and out's of request handling -- is there any form data that is non json that could need additional handling? My understanding is that Hanabi will handle most forms automatically.
- add Inferno::Utils::NamedThorActions - add dry-inflector dependency - add Inferno::CLI::New thor generator - add lib/inferno/apps/cli/templates/ folder with ERB templates for new Inferno test kit --------- Co-authored-by: Alyssa Wang <awang@mitre.org> Co-authored-by: Stephen MacVicar <Jammjammjamm@users.noreply.github.com>
| @@ -10,7 +10,9 @@ class Create < Controller | |||
|
|
|||
| def handle(req, res) | |||
| params = req.params.to_h | |||
| params.merge!(JSON.parse(req.body.string).symbolize_keys) unless req.body.string.blank? | |||
| if req.body.string.present? && req.env['CONTENT_TYPE'].include?('application/json') | |||
There was a problem hiding this comment.
| if req.body.string.present? && req.env['CONTENT_TYPE'].include?('application/json') | |
| if req.body.string.present? && req.env['CONTENT_TYPE']&.include?('application/json') |
* add titles to infrastructure test suite external groups * add rspec tests * fix server side error for empty group * rubocop conformance
…o-framework/inferno-core into FI-2318-Session-form-POST
This reverts commit 5c35cf6.
Summary
Added a guard in the
test_sessioncreate endpoint so it only parses as a json when the request does not contain form-data.Testing Guidance
Added a spec test in
spec/requests/test_sessions_spec.rbas well as a helper function to setup the form data request. Requesting a closer look at these to make sure I set them up correctly, as I'm still picking up the ins-outs of spec testing the more web-facing parts of the project.