Consumer being the owner of the HoldRequestResult stream

[holingpoon]

Hello, as per conversation on the new architecture, the following is a sample of what the HoldRequestResultConsumer is expecting. This is subject to change. Please post here for proposing the new data model/schema. Thanks!

  "Records": [
    {
      "jobId": "901bdd1d-bd8f-4310-ba31-7f13a55877fd",
      "success": false,
      "message": "This item is already taken.",
      "holdRequest": {
        "id": 229,
        "jobId": "901bdd1d-bd8f-4310-ba31-7f13a55877fd",
        "patron": "2320818",
        "nyplSource": "sierra-nypl, recap-nypl, recap-cul, recap-pul",
        "createdDate": "2016-01-07T02:32:51Z",
        "updatedDate": "2016-01-07T02:32:51Z",
        "success": true,
        "processed": false,
        "requestType": "bib, item, edd",
        "recordType": "b, i, j",
        "record": "17388176",
        "pickupLocation": "sasb",
        "neededBy": "2016-01-07T02:32:51Z",
        "numberOfCopies": 1,
        "deliveryLocation": "SASB",
        "docDeliveryData": {
          "emailAddress": "holingpoon@nypl.org",
          "chapterTitle": "Chapter 10",
          "volume": "158",
          "issue": "Summer 2017",
          "startPage": "209",
          "endPage": "259"
        }
      }
    }
  ]
}

[kfriedman]

Given that multiple people are going to post to the stream now, I don't think we can (or should) include the entire HoldRequest. We might need to change it to:

{
  "Records": [
    {
      "jobId": "901bdd1d-bd8f-4310-ba31-7f13a55877fd",
      "success": false,
      "message": "This item is already taken.",
      "holdRequestId":  229
    }
  ]
}

Thoughts?

[rhernand3z]

I agree with not including the entire HoldRequest Object. However, for failed requests, I propose we use something like this:

{
  "Records": [
    {
      "jobId": "901bdd1d-bd8f-4310-ba31-7f13a55877fd",
      "success": false,
      "error": {
         message: "This item is already taken.",
         code: 'some unique code here' // Well documented by person responsible for this service.
       },
      "holdRequestId":  229
    }
  ]
}

We could swap 'error' with 'details'

I feel the front-end or anyone consuming this response would easily add logic to check success === 'false' and possibly handle all types of error-codes elegantly versus parsing the message string and figuring out what happened.

Thoughts? @kfriedman @holingpoon @gkallenberg @emu47

[gkallenberg]

The trimmed down model should be sufficient to update the hold request via the hold request service API. Although, we may want to add a "processed" element unless the responsibility of setting that to true after the creation state of false lies with the hold request service itself.

??

Processed gets set after the service bus success response?

--

Gregory Kallenberg | The New York Public Library Senior Applications Developer

Digital Development 40 West 20th Street, 5th Floor, New York, NY 10011 T: 212.621.0283 | gregorykallenberg@nypl.org http://www.nypl.org Inspiring Lifelong Learning | Advancing Knowledge | Strengthening Our Communities

On Fri, Jun 16, 2017 at 2:16 PM, kfriedman notifications@github.com wrote:

Given that multiple people are going to post to the stream now, I don't think we can (or should) include the entire HoldRequest. We might need to change it to:

{ "Records": [ { "jobId": "901bdd1d-bd8f-4310-ba31-7f13a55877fd", "success": false, "message": "This item is already taken.", "holdRequestId": 229 } ] }

Thoughts?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/NYPL/hold-request-result-consumer/issues/3#issuecomment-309097553, or mute the thread https://github.com/notifications/unsubscribe-auth/ABpZWUnHbLxSW_5t7n4_u9TrebynSzPwks5sEsZ0gaJpZM4N8xQL .

[holingpoon]

@rhernand3z I would vote for details over error since that's a better fit for both success===true and success===false. I am just wondering whether the code should be numeric. Card Creator passes back type values such as valid-address and available-username. As long as there's an agreement of what the consumer should look for and the code values are unique, I think we will be okay.

[kfriedman]

code sounds like a good idea to me. Or maybe type is better like @holingpoon suggested? Though, I think we're in a slightly different position than Card Creator where we have multiple publishers of the data. So, who is the authority on type here? And who is responsible for ensuring the documentation is accurate?

I see your point @holingpoon about renaming error to details. Though maybe we should just keep things simple and assume success === true has no message and act accordingly? Otherwise, we'd have to account for a success === true message and figure out how to handle. I'm not sure what that would look like.

Good question @gkallenberg. Yeah, I think like you said, the responsibility of setting processed to true lies with the Hold Request Service.

So, is it shaping up to the following?

{
  "Records": [
    {
      "jobId": "901bdd1d-bd8f-4310-ba31-7f13a55877fd",
      "success": false,
      "error": {
         message: "This item is already taken.",
         type: "some-unique-code-here" // Well documented by person responsible for this service.
       },
      "holdRequestId":  229
    }
  ]
}

[holingpoon]

@kfriedman I see what you mean by only looking for error when success === false. My understanding is when success === true, I need to do a lookup from holdRequestId for the holdRequest, then look up patron and item (eventually bib) based on the ids provided by the holdRequest.

[kfriedman]

I think your understanding is correct @holingpoon.

[holingpoon]

Closing this issue and using the following as schema

  "Records": [
    {
      "jobId": "901bdd1d-bd8f-4310-ba31-7f13a55877fd",
      "success": false,
      "error": {
         message: "This item is already taken.",
         type: "some-unique-code-here" // Well documented by person responsible for this service.
       },
      "holdRequestId":  229
    }
  ]
}

[holingpoon]

@emu47 This is where we last left off for data format to be published to the HoldRequestResult data stream, see comment https://github.com/NYPL/hold-request-result-consumer/issues/3#issuecomment-310165215. Let me know if you have any questions. Thanks!

cc: @kfriedman @gkallenberg @rhernand3z