MB-2737 Ignore service item params in CreatePaymentRequestPayload

[noelledusahel]

Description

The Prime may attempt to send Service Item Params through the Create Payment Request payload, but we want the source of truth to be the MTO. This PR achieves ignoring the input of service item params by converting this part of the payload to readOnly, and removing the handler function that assigns serviceItemParams to serviceItems. At this time there can be a CreatePaymentRequest success when serviceItemParams are passed, without throwing an error.

Reviewer Notes

Is there anything you would like reviewers to give additional scrutiny?

Setup

• make db_dev_e2e_populate
• make server_run
• Make a pr_payload.json file containing the following (which references E2E data) running with this payload should produce an error, to make it successful, then remove the params part of the payload:

  {
    "body": {
      "isFinal": false,
      "moveTaskOrderID": "9c7b255c-2981-4bf8-839f-61c7458e2b4d",
      "serviceItems": [
        {
          "id": "76055c99-0990-410c-a7c9-69373b0b53eb",
          "params": [
            {
             "key": "weightActual",
              "value": "1111"
            }
          ]
        }
      ]
    }
  }

• go run ./cmd/prime-api-client --insecure create-payment-request --filename pr_payload.json | jq
• Verify parameters and that the payment request is created without the service item params passed in through the payment request payload (in this case weightActual).

Code Review Verification Steps

• Code follows the guidelines for Logging
• The requirements listed in
  Querying the Database Safely
  have been satisfied.
• Any new migrations/schema changes:
  • Follow our guidelines for zero-downtime deploys (see Zero-Downtime Deploys)
  • Have been communicated to #g-database
  • Secure migrations have been tested following the instructions in our docs
• There are no aXe warnings for UI.
• This works in Supported Browsers and their phone views (Chrome, Firefox, IE, Edge).
• Tested in the Experimental environment (for changes to containers, app startup, or connection to data stores)
• User facing changes have been reviewed by design.
• Request review from a member of a different team.
• Have the Jira acceptance criteria been met for this change?

References

• Jira story

Screenshots

If this PR makes visible UI changes, an image of the finished UI can help reviewers and casual
observers understand the context of the changes. A before image is optional and
can be included at the submitter's discretion.

Consider using an animated image to show an entire workflow instead of using multiple images. You may want to use GIPHY CAPTURE for this! 📸

Please frame screenshots to show enough useful context but also highlight the affected regions.

[robot-mymove]

	Messages
📖	🔗 MB-2737

Generated by 🚫 dangerJS against 96e659d

[gidjin]

Looks like we should also return an error if params are sent in since they are now readOnly. See this thread. Also in trying it I get an error when running the steps in the setup, not sure if I've missed something or not.

2020/06/30 02:48:13 File decode failed: json: unknown field "params"

[gidjin]

Is there a way to also verify that the call to CreatePaymentRequest isn't given any ServiceItemParams? I think that this will just be checking the mock return value that you defined above.

[jacquelineIO]

@gidjin looks like this as addressed already.

[gidjin]

Is there a way to test the error message that would be returned if someone were to pass in params?

[jacquelineIO]

@gidjin yes, I expect when the params are sent, there will be an error message. Or do you mean to test this in unit test? I think there is test for this but not on this function... let me see

[gidjin]

Unit test or integration test. Just some sort of test, if possible would be nice. If not no worries

[pearl-truss]

I'm getting a no matching versions for query "latest" after running make db_dev_e2e_populate. I did a make clean after switching branches

[blvrd]

I'm getting a no matching versions for query "latest" after running make db_dev_e2e_populate. I did a make clean after switching branches

@pearl-truss here's what I usually do before reviewing a pull request:

make clean && make build && make db_dev_e2e_populate && make server_run

Maybe that will solve the problem. Also, every once in a while, I run make prune to clear out old Docker volumes just in case something weird is going on there.

[blvrd]

This all looks great, and I'm getting the proper error when trying to send in service item params. I do agree with @gidjin that it might be good to test the actual error message that is returned if you have time to add that.