FI-2015 GET requests added to fhir_operation

[360dgries]

Summary

According to HL7 r5 specification:
Operations may be invoked using a GET, with parameters as HTTP URL parameters, if:

1. there are only simple input parameters - i.e. no complex datatypes like 'Ratio' or 'Reference'
2. the operation does not affect the state of the server

fhir_operation now has an optional operationMethod argument that indicates whether GET or POST should be used (defaulting to POST).
Discovering affectsState was considered out of scope for this ticket -- as such, it is currently up to the caller to know about the Operation's affectsState value before determining whether they can use a GET request.

Any parameters are checked to ensure they are primitive, then converted to a query string that is appended to the path given to fhir_operation.

Tests added

• GET request is made when there are no parameters and GET is specified
• GET request is made when there are only primitive parameters and GET is specified
• GET request prevented when non-primitive parameters are in the body
  • Both with a complex parameter and a resource as a parameter

I've also included a commented out test for handling repeated parameters -- this is something that is handled through the call to fhir_client and Stephen suggested spinning off another ticket, FI-2223, and I have left details in the description of that ticket for using the test.

[codecov]

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (426ab2c) 76.97% compared to head (cd511f2) 77.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #380      +/-   ##
==========================================
+ Coverage   76.97%   77.00%   +0.03%     
==========================================
  Files         214      214              
  Lines       10693    10708      +15     
  Branches      986      991       +5     
==========================================
+ Hits         8231     8246      +15     
  Misses       1884     1884              
  Partials      578      578              

Flag	Coverage Δ
backend	94.35% <100.00%> (+0.02%)	⬆️
frontend	69.75% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
lib/inferno/dsl/fhir_client.rb	99.06% <100.00%> (+0.15%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[360dgries]

Currently have two blocks of let statements that are identical in the tests for body_to_path and fhir_operation. Let me know if I should move them to the top where let(:group) is since they are used by multiple test groups.

Also, let me know if body_to_path should just return the converted body, and not receive the path as an argument and append the converted body.

[Jammjammjamm]

Currently have two blocks of let statements that are identical in the tests for body_to_path and fhir_operation. Let me know if I should move them to the top where let(:group) is since they are used by multiple test groups.

Yes, move common lets up as high as you can.

Also, let me know if body_to_path should just return the converted body, and not receive the path as an argument and append the converted body.

Absolutely return the converted body rather than modifying an argument.

[Jammjammjamm]

This branch isn't doing anything javascript-related, so package-lock.json should not be changing.

[360dgries]

This branch isn't doing anything javascript-related, so package-lock.json should not be changing.

I believe Alisa ran into similar issues as myself upon setup, and found a temporary fix in deleting package-lock.json. I bet I did the same to avoid some of the npm install issues -- I'll revert the change and see if the tests still run fine, then make another push.

[360dgries]

Changes:

• Changed safe_for_get? to do much less. Now just strips the name and checks the value for being primitive
  • It still uses FHIR.primitive? for validation. I know we talked about not using .valid? -- is this a similar case?
  • Has had its name changed to primitive_parameter? to better reflect what it is doing.
  • We discussed the behavior for this when it is given a non-primitive -- I think it makes sense for this to just be a boolean-providing function and have any error handling in other methods that use it, but let me know if this should be a "provides true or errors" method.
• rspec tests reorganized to be mostly under fhir_operation
  • Handling of repeated parameters tested only at body_to_path level, and handling at the request level deferred to ticket FI-2223

[360dgries]

I'm running into an issue running bundle install locally, and looking into it it looks like this has been an issue on windows in the past -- can a Mac user pull and try to run? Here is the error I'm getting:

NoMethodError: undefined method `full_name' for nil:NilClass

  warning << "* #{unmet_spec_dependency}, depended upon #{spec.full_name}, unsatisfied by #{@specs.find {|s| s.name == unmet_spec_dependency.name && !unmet_spec_dependency.matches_spec?(s.spec) }.full_name}

[Jammjammjamm]

This branch shouldn't be changing Gemfile.lock, package.json, or package-lock.json.

[360dgries]

How should I resolve this? Is copying the current state of these files from main and moving them over to this branch enough?

I'm also wondering if these changes are brought about from running bundle install with a different node/npm version -- I think we had a discussion on slack about that at one point but not sure if it applies here?

[Jammjammjamm]

Is copying the current state of these files from main and moving them over to this branch enough?

Yes, you could do that. You should be able to tell which commands are changing those files by running the commands and seeing what changes.