redcross / smoke-alarm-portal

Red Cross web portal accepting free smoke alarm installation requests
https://getasmokealarm.org/
Other
7 stars 20 forks source link

API-Level Integration #196

Open MisterJames opened 8 years ago

MisterJames commented 8 years ago

Hello all,

I'm a contributor on another open source project used by the Red Cross (AllReady) and I'm looking for some information regarding integration with the API.

For certain regions, we are looking to either receive a post-back when a new smoke alarm is detected (preferred) or periodically poll an end point to "pick up" new requests. Any information that you might have would be greatly appreciated.

We are implementing a web hook approach on our site for implementation of such communication, so it would be trivial for us to expose a URI and give you a token to use as part of the post-back, should that work for you. Perhaps this is something that could be registered with a list of cities, states, or geo-coded info of some kind such that only requests for the designated areas are presented to our end point.

If you don't have direct support for this type of integration, we can also use services such as IFTTT or Zapier and consume events from those providers.

Contacts that I am working with:

Looking forward to working with you folks!

Cheers.

jim-mcgowan commented 8 years ago

@MisterJames, @kfogel and @cecilia-donnelly: I'll facilitate introductions here. James Chambers is more than a contributor to allReady. As a volunteer and project leader, he is one of the main drivers of allReady's success to date. The fact that he works from Manitoba, Canada only adds to his allure! Karl Fogel and Cecilia Donnelly are 2/3 of Open Tech Strategies, great folks here in Chicago who have served as my "digital sherpas" as we venture into the world of OSS. Virtual handshakes all around. So happy to have you all connected!

kfogel commented 8 years ago

Hey, @MisterJames. Looking forward to working with you too! This all sounds technically straightforward; I'll be able to send a more substantive response tomorrow (am about to head out tonight), but don't anticipate any problems.

(virtual handshake)

cecilia-donnelly commented 8 years ago

Hey @MisterJames, Thanks for reaching out! What you're describing sounds very reasonable. Just to make sure we're on the same page, I'm imagining a workflow as follows:

  1. We receive a request for a smoke alarm.
  2. We store it in our db, as usual, and also POST it to you, along with an authorization token that you've provided to us. We're using HTTPS, by the way, and presumably you are too.

    • You mention limiting the requests sent to AllReady by some kind of geographical info. We assign each new request to a Red Cross region, so could easily limit the requests we send to AllReady to only include those in active regions (i.e., regions currently using the portal to accept smoke alarm installation requests).
    • The fields we store for each request are:

      name, // the name of the person requesting the alarm
      address, // their address
      assigned_rc_region, // the Red Cross region associated with their zip code
      city,
      state,
      zip,
      phone,
      email,
      serial, // our unique identifier: Red Cross region code - 2-digit fiscal year - integer
      status // currently: new, in progress, installed, or canceled

      Which of these fields would you like to receive? All of them? The serial field can be used as a key back into our database, in case as a future enhancement we want to send updates to AllReady about the request -- e.g. whenever the status changes.

    \3. You POST back some confirmation code to an endpoint that we make available. We then store that code along with the request.

    Is that consistent with what you were thinking?

MisterJames commented 8 years ago

@cecilia-donnelly Yes, that sounds pretty much in line. Thanks for this detail.

For regions:

I think we will need to introduce the concept of associated region (or some kind of org identification) in our model. This would allow us to test if we support it. I think we would return a HTTP 202 for your post, but then we'd need to let you know somehow that we weren't covering that region.

In instances where we are going to service the request, what status should we send you back? It would seem that the status list you have is fine, but perhaps if there were a flag on the request (third_party_tracked or something) we could set that to true and maintain the status for you as they are currently defined. Thus, an endpoint where we could post a status would be suffice (because we could send the tracking flag, and the status). Here are some ideas of how that might work:

Does that jive? Perhaps @OhMcGoo has some thoughts here as well for how the tasks/installs/statuses might align.

I'm not involved in the deployment process, but I will make sure with our folks that we're on HTTPS before we start exchanging data. Would you also have an auth token for your endpoints that we should use? We can exchange that off-thread of course ;) :dancers:

Cheers!

MisterJames commented 8 years ago

Hey folks! Just checking in to see if there is any progress, or if we should perhaps set up a quick call sometime this week to help move this one along. Have a great weekend! cc/ @cecilia-donnelly @OhMcGoo

kfogel commented 8 years ago

Hey, @MisterJames. Sorry for the delay; we're doing some prioritization with @OhMcGoo (a few different features in this project, and one other project entirely). That's why you haven't heard back yet. We will try to get this all sorted out with Jim as soon as we can and make this happen. The technical plan as outlined above looks pretty darned sane to me.

Yes, you found the list of supported Red Cross regions (though I would recommend using this live link so that you always have the most up-to-date version of the file). However, your suggestion that we just add an API to provide the supported regions, with effective date provided, is the right solution. We don't have that API yet, but could certainly add it.

Yes, we'd give you an auth token(s) via secure channel, that you would use to authenticate to our API.

For the list of statuses that you send back, we might (?) want to add one more:

I'd like to get implementing this. We just need to do some planning with Jim to get this on the front burner. Watch this space for more.

cecilia-donnelly commented 8 years ago

Hey @MisterJames. We've talked to @OhMcGoo and will start work on this next week. #202 is especially time-sensitive and so we'll get started on that first, but expect to see movement in this ticket in the next few days. I imagine we'll have more questions for you as we go -- thanks for getting us going on this.

cecilia-donnelly commented 8 years ago

Hey @MisterJames and @OhMcGoo -- I'm starting on this today, now that I've done some work on #202. I'll ping here if I run across problems/questions.

MisterJames commented 8 years ago

Nice! Thanks for the effort here...I'll try to work through my end as expediently as possible so we can start getting these bits chatting.

kfogel commented 8 years ago

Hey, @MisterJames. Let us know when you've got some people working on the corresponding endpoints on the AllReady side -- we're continuing to test here, and looking at tweaking a few aspects of the API design, but we're certainly ready for those conversations and even ready to hit endpoints as soon as they're up.

cecilia-donnelly commented 8 years ago

@tonysurma points out that AllReady wants to avoid having Red Cross business logic in their app as much as possible. He suggests that instead of sending the Red Cross-specific region with the request, AllReady may give us several different tokens, one per region. AllReady would process requests by token, instead of sorting them by region. This way, they could handle all tokens from all external clients the same way, instead of having a particular section of code to sort requests across regions.

Note that the following work has been done on the AllReady side (from conversation in a Slack channel):

@stevejgordon: A lot of the initial work was done by @MisterJames - Check out PR's #802, #964, #966 and #990 stevejgordon: I'm currently working on some code for listing requests for a particular event and following that probably some code to allow manual entry/editing of the requests. Probably won't overlap too much with any API work. stevejgordon: We have a request status enum as well and would expect any new requests to be in the unassigned status initially. @seanepping: I am currently looking at Issue #1055 stevejgordon: The idea is then that an organizer can assign those requests into an itinerary. Ah okay, that sounds great. I shouldn't need to change the request object in anything I'm working on so hopefully no merge conflicts later on.

Based on further conversations with them, we probably want to:

  1. Get multiple tokens from them, one per region. Would these be static?
  2. Send the correct token with our request, based on the region it is in. This might have something to do with #207 -- in my mind, the more tokens we have the more it makes sense to keep them in the database. Thoughts, @kfogel?
mgmccarthy commented 8 years ago

@cecilia-donnelly cc @kfogel @MisterJames

I've been taking up the work on the AllReady side for some parts of this integration. I had a lot of questions on the AllReady side, but after finding this, it sounds like a good amount of my questions have been answered. I'd just like to confirm a couple things:

Sorry if some of this is already known, but I did just pick this up a while ago, so any details you can provide me with the questions above should help me write the correct code on AllReady's side as well as break down our Issues for this integration into more granular chunks so we can prioritize them.

Thanks!

cecilia-donnelly commented 8 years ago

Hi and welcome, Mike! (@mgmccarthy)

  • it sounds like each incoming request will have a unique identifier, the serial column. Above is an example: "serial, // our unique identifier: Red Cross region code - 2-digit fiscal year - integer". Am I correct to assume that this will individually identify each and every request you sent to us? The only time we should every receive the same serial value more than once is the case where we're processing a status update, correct?

Yes, exactly.

  • i'm making the assumption that once a region code is assigned, it never changes, as it's part of what makes up your field for the unique identifier per request.

Yes, we're making that assumption too. It's possible that the regions might change in the future, of course, but each request will be assigned to the region corresponding to the requester's address when the request is received.

  • do we return the serial value to you with our reply so you can correlate back to your system? In other words, what do you need back, the entire message, or the serial with the status codes/http responses?

Yes, we need the serial number back. @MisterJames' comment here https://github.com/redcross/smoke-alarm-portal/issues/196#issuecomment-213773653 suggests that you'd send the serial, the status, and a boolean indicating whether or not the request will be tracked in AllReady.

  • it sounds like these are the possible four "statuses" you can send us: new, in progress, installed, or canceled. we can map these statuses to our internal AllReady statuses.

Yes, though we aren't yet enforcing those statuses on our end. If some different or expanded set would be better for you, let us know!

  • how is the auth token given to getasmokealarm.org? Is this something our system provides to you via code/endpoint, or is it a auth token we give to you by some other means?

I think this is up to you.

  • do you know if any headway was made on supplying a token per region?

Not as far as I know.

  • sounds like we have yet to reply to your request with the updated status/httpcode as well as testing out the API/auth token code, correct?

Right. I wrote up some docs for the incoming endpoint here: https://github.com/redcross/smoke-alarm-portal/blob/196-api-integration/docs/API.md, in case you haven't seen those.

I'm glad to hear that you're picking this up. Let me know if you need more detail or have other questions!

mgmccarthy commented 8 years ago

@cecilia-donnelly, thanks so much! Right away, with the information you gave me, I have some changes to make to my initial take on this integration.

I'll talk with @MisterJames about the auth token. I believe something has been written on our end, but I might now know about it yet.

mgmccarthy commented 8 years ago

@cecilia-donnelly I have a couple more questions for you.

I understand getting a request from you that has a status of "new", but I'm not too sure how your system gets updated to the other three statuses? When the request comes into your site, and you persist it, what actions occur through your site that would change the status of those requests? Do all request updates get funneled back through your API and allReady just needs to track the current status in the context of what our application does?

For example, in this case, a smoke alarm install. Eventually, someone is going to show up and install a smoke alarm somewhere. When the installer finished the job, do they mark the install complete through your site? Or does your site just allow the updating of a status via the API, and allReady tracks that type of information?

cecilia-donnelly commented 8 years ago

That's a great question. Currently, users can update the request status on our site but we don't have anything set up to send those updates to AllReady. Let's finish sending requests to you first and then add these updates.

Our site does allow a status update via API from AllReady.

Maybe @OhMcGoo wants to chime in on this. Do you anticipate users updating request status through AllReady, the smoke alarm portal, or both?

mgmccarthy commented 8 years ago

@cecilia-donnelly

Currently, users can update the request status on our site but we don't have anything set up to send those updates to AllReady

Since allReady will be tracking the status of the request internally (if we accept the request), I'm assuming that any request updates once in our system would be funneled back through your API to update the status of the request on your side.

If we do anticipate having to update statuses that we initially received from you, and we're trying to figure out what updates to send back to you, this could get a little confusing.

It sounds like you will receive status updates from your users about smoke alarm install, but who is the "user" doing the updating in this case? Is it the volunteers installing the smoke alarm, or, for example, the resident(s) of the home that initally requested the smoke alarm via your portal?

I'm asking this b/c allReady's job is to coordinate the right volunteer(s) with the right skills on the correct day for the install... and our site is built to track that information. So... I'm thinking that once we receive a new request from you, we either would not want to process status updates for those requests, but rather, report status updates from our system (driven by volunteers) to your side via your API.

It would be good to get a little feedback here from @OhMcGoo cc @MisterJames

stevejgordon commented 8 years ago

@mgmccarthy I've just spotted this. I'm a bit weighed up with work (so not read it all properly yet). I do have an API spec @MisterJames forwarded from @cecilia-donnelly describing how allReady can call to getasmokealarm to update statuses. I was going to look at creating a little service/handler for that which will fire when a request status changes at our end.

I did probably have some questions but don't have the email in front of me at the moment

mgmccarthy commented 8 years ago

thanks for weighing in @stevejgordon. You can also refer this link for how to send update to getasmokealarm's API: https://github.com/redcross/smoke-alarm-portal/blob/196-api-integration/docs/API.md.

if you have something more concrete, can you share it with me on Slack?

Also, these questions I'm asking are not because I don't understand the API part of what we're building here, but rather, do we need to/want to update our internal statuses as a result of a request we receive from getasmokealarm that is not "new"?

Changing our internal Request status is not as simple as just updating a field, but rather, there are db updates involved and command/notifications we dispatch based on the Request status changing, basically, we have a workflow in place.

What I'm looking for here is the "source of truth". Does allReady just process the "new" requests from getasmokealarm and then publish out when our requests changes to getasmokealarm's API? If that's the case, then this is easy and makes sense.

However, if we need to handle updates from getasmokealarm (for instance, they send us a request update with "in progress"), and then make sure allReady is updated appropriately, it's not as simple. Also, in this example, would we even need to publish an update out to their API b/c we received the status change from them and not from something in allReady?

stevejgordon commented 8 years ago

@mgmccarthy Agreed, if we need to keep in sync with changes from getasmokealarm after the initial request creation it will be much more complex.

My original impression was that once in allReady we would own the status and update back. getasmokealarm would be mainly the portal to accept the initial requests. Like you, I agree we need to confirm the requirements/expectation.

cecilia-donnelly commented 8 years ago

What I'm looking for here is the "source of truth". Does allReady just process the "new" requests from getasmokealarm and then publish out when our requests changes to getasmokealarm's API? If that's the case, then this is easy and makes sense.

This is the simplest way forward in my opinion. It's best to just have one master, as @mgmccarthy says. @OhMcGoo, is that how you imagined this working? @kfogel, want to chime in?

cecilia-donnelly commented 8 years ago

Hey @mgmccarthy, @stevejgordon, @MisterJames how's it going? Do you need anything else from me at the moment?

mgmccarthy commented 8 years ago

@cecilia-donnelly, @stevejgordon and I still need to talk to @MisterJames about how we're accepting status updates (allready/portal/both) and what that means to our systems trying coordinate tracking the "status" of a request.

MisterJames commented 8 years ago

So, after a few internal and cross-team conversations I'd just like to sum up our understanding and, as @mgmccarthy said, we can get this over the finish line!

@cecilia-donnelly thanks for your patience and continued efforts here, much appreciated!

Here's what we believe to be true:

Cecilia, if you're good with that and it matched @OhMcGoo's expectations, I think the code pieces can fall in line pretty quickly.

mgmccarthy commented 8 years ago

@cecilia-donnelly, I'll be fixing some stuff up on our end this week to account for the convo with @MisterJames. Either I or @stevejgordon will get back to you with that part where we invoke your API to update a status.

Basically, the only requests we'll accept from your portal will be "new" requests. As those requests move through AllReady's request life-cycle, we'll be invoking your API to update status back to you.

Thanks!

mgmccarthy commented 8 years ago

@kfogel... from this comment above: https://github.com/redcross/smoke-alarm-portal/issues/196#issuecomment-216733389

However, your suggestion that we just add an API to provide the supported regions, with effective date provided, is the right solution. We don't have that API yet, but could certainly add it.

did you ever get around to building out an API to expose the list of regions?

cecilia-donnelly commented 8 years ago

@MisterJames, @mgmccarthy that sounds great!

This means that we need to:

From you, we need:

And @mgmccarthy, yes, there is an API for the list of regions. Do a GET request to /regions on the demo server, and let me know if that works. I'll need to add auth to that endpoint -- see https://github.com/redcross/smoke-alarm-portal/commit/8c56b46b7e074d897b02dfa85275725c1f8fa926 for more details.

mgmccarthy commented 8 years ago

@cecilia-donnelly cc @MisterJames cc @kfogel

since we will only accept requests from getasmokealarm that have a status of "new", you might not have to write any code to forbid admins to edit the status of a request that is owned by AllReady, we would just ignore the request. I'm not too sure what it involves from your end, or if adding that type of vendor specific code is something you mind doing.

I will provide you with a destination endpoint address soon, but I actually need to get the code done on my end first, b/c what we have in our live server is incorrect :wink:

Also, I'd like to bring up two things:

how we provide you the "acknowledgement":

@MisterJames suggested returning a 202 http status code which would mean the call was accepted, but we need to figure out how to get information back to you as to whether we service that request or not. I'm not too sure if you want/require an ack back immediately (RPC/locking call) if something like the "serial" field was missing, or there was some other malformed info in the message, or if just the 202 is fine.

OR we could instead communicate back through your API that we did not accept the request you sent us. We could use this approach for every kind of reason that we could not accept a request; malformed request, information missing from the request, unsupported region, etc....

I like the second approach b/c there is no locking element to the call, which keeps communication from "hanging"..

Please let me know what you need back from us at the point the call is returned to you.

assigned_rc_region mapping

There are discussions above about two ways to handle regions:

  1. internally, we add some type of region identifier to one or more of our internal entities as use that to determine whether or not we can service the request based on region
  2. issue multiple tokens to getasmokealarm, one for each region that we support. As we support more regions, we'd have to get your more tokens, and you would need to make sure the correct token goes out on the request for a given region. The tokens add an abstraction to the look-up for region where we don't any any RC specific code to our core domain.

I have a question out to @MisterJames on which approach he would prefer for our initial launch, or if there is something else we can come up with.

Again, I'd like to here your thoughts.

mgmccarthy commented 8 years ago

@cecilia-donnelly @kfogel cc @MisterJames

I'm curious about how your regions codes work. Are they somehow tied to zip codes? Is there some association between them that getasmokealarm stores?

Looking here it seems zip codes might be a subset of region codes?

Example; what zip codes are in rc_western_missouri and rc_eastern_missouri?

Thanks

mgmccarthy commented 8 years ago

update on this comment.

We're going to stick with one token per "provider" instead of multiple tokens for each region.

As soon as I can finish up/get the token generation stuff working, I'll be submitting a PR, then give you guys an example of what the json request will need to look like in order to invoke the endpoint.

Thanks

cecilia-donnelly commented 8 years ago

Hey @mgmccarthy, I'm excited to see what you come up with! Definitely link us to the PR when it comes.

For region codes: Red Cross regions follow county boundaries. We associate counties with regions in one table (see here), and then associate zip codes with counties in another table. So, when a request comes in we use its zip code to find which region it belongs to (we call this from here).

So, to find out what zip codes are in the western and eastern MO regions, I'd run a query in the database joining the SelectedCounties and UsAddress tables. Does that help? (I'll follow up with a list of zips but I think it will be pretty long.)

mgmccarthy commented 8 years ago

cc @MisterJames

@cecilia-donnelly, thanks for explaining how countries, regions and zip codes work together. You don't need to provider us with a list of zips.

So, based on what you're telling me, what we'd need to be able to do is to determine on our end is if any of our organizations support the zip code your sending us on the request.

I'm starting to think it might be as simple as that (unless I'm still missing something). You're sending us both the zip and assigned_rc_region in the request, so what we could do it look to see if any of the our existing organizations fall into that zip code. That being said, I'm not too sure how/why we would even need the assinged_rc_region in the first place

This assumption is based off of you only sending us a request where you'd be able to correlate the incoming zip code to the region on your end. If you can't make that association, I imagine you would not invoke our API?

I'd like to keep AllReady's code-base out of the country/region/zip code look up b/c you already do that on your end and just optimize it for processing requests that its organizations can service.

Thanks

mgmccarthy commented 8 years ago

@cecilia-donnelly, we have an endpoint live! cc @MisterJames

Right now, we do NOT have our token based auth ready, but you can test this endpoint by going to the following URL: http://allready-d.azurewebsites.net/api/requestapi

and sending us a json request that looks like this:

{
  "ProviderRequestId":"[this would be your serial field]",
  "ProviderData":"[this would be your assigned_rc_region]",
  "Status":"new",
  "Name":"RequestName",
  "Address":"RequestAddress",
  "City":"RequestCity",
  "State":"RequestState",
  "Zip":"RequestZip",
  "Phone":"111-111-1111",
  "Email":"email@email.com"
}

you should receive a 202 http status code back. If you receive a bad request response back, let me know, and I can troubleshoot on this end.

Right now, we do not have the API call back to your endpoint to update status written yet, but we're working on that right now.

I'll need some answers as to what you expect back right away on the 202 (acknowledgement) vs. if we can communicate any reason (missing/invalid message content, un-supported region, etc...) for a message to fail back to your API.

We're also still figuring out how we're going to make a decision about whether not not we can service the request on our end given the region and the zip code you're passing us.

Thanks

cecilia-donnelly commented 8 years ago

Hey @mgmccarthy,

This assumption is based off of you only sending us a request where you'd be able to correlate the incoming zip code to the region on your end. If you can't make that association, I imagine you would not invoke our API?

That's right! I've been planning to send you our object more or less in full, even though you might not need the assigned_rc_region element. I can change that, though.

you should receive a 202 http status code back

I just did a quick test using your example JSON with a serial and a region code (via the Postman app) and I did indeed get a 202 response from that endpoint. Thank you! As far as receiving more information back from you than just an acknowledgement, I've been expecting something along the lines of what @MisterJames explained upthread: https://github.com/redcross/smoke-alarm-portal/issues/196#issuecomment-213773653.

I'll get moving on sending test information to that endpoint. Exciting!

mgmccarthy commented 8 years ago

@cecilia-donnelly

As far as receiving more information back from you than just an acknowledgement, I've been expecting something along the lines of what @MisterJames explained upthread: #196 (comment).

some of the "checks" we're going to perform to the incoming message will be more "involved" than others. There two types of checks we'll perform:

field validation

this is validation like required fields missing we need in order to create a Request in our system, validating the phone number and email are in a valid format, etc... basically, run-of-the-mill type validation. We could also include missing or incorrect token data in the http header for the incoming request

This type of validation is very inexpensive to perform and it's quick.

business validations

this is where we start to get into scenarios like:

This type of validation often has a cost to it, mainly a database query or queries that we need to do in the transaction of the our endpoint being invoked. These type of validations are often more costly to perform and could be slower.

We're going to return to you an HTTP status code of 202. Looking up what a 202 actually means, I found this:

The request has been accepted for processing, but the processing has not been completed. The request might or might not eventually be acted upon, as it might be disallowed when processing actually takes place. (https://httpstatuses.com/202)

Based on that definition, I'm assuming the actual outcome of the 202'ed response (whether or not we service the request or not) would be communicated back to you via your API, and not in the response to your request.

So, my question is, how do you want us to handle validation failures (both field and business) during the invocation of our endpoint? Some ideas:

Looking at your API, it seems you allow us to communicate two things back to you:

Assuming we will report any field/business validations back to your API after we give you a 202, what would some possible combinations of acceptance and status be for:

I'm sorry this is so long, but since I've never returned a 202 before, and since you seem to have an API we invoke to communicate back to you, I don't know what you expect back from us on the response that contains the 202, and if we can also return things like 400's.

mgmccarthy commented 8 years ago

@cecilia-donnelly, looking at us_addresses.json, and it looks like you have latitude and longitude information.

Could you please add that to the data you're sending us? We need longitude and latitude. If it's missing ,we are using a geo codeing nuget package on our end to try to figure out the long and lat. It would probably be better if you could send it along on the message.

Thanks!

cecilia-donnelly commented 8 years ago

@mgmccarthy, thanks for the detailed response!

First, about lat/long: we don't have it for the requests (unfortunately). All we have is the address the user sends us. The latitude and longitude in UsAddress refers to the zipcode in that table, not to any request in that zipcode.

Now for the interesting part:

field validation

since field validation is simple/fast, any field validation errors we could reply back to you on the response of the request you make to the endpoint with a 400 http status code. I don't know if your code is listening for a response to this request, and if it is, if you do anything with that response.

I like the 400 idea! The code we have so far to handle sending you a new request is here (in the PostRequest function). It's a work in progress, since I didn't want to go too far without an endpoint to test with on your end. Right now I'm just logging any response that isn't a 200 or 202, but I could definitely listen for field validation errors there.

So, to look at your specific examples:

  • field was missing
  • email is not valid

These would then both get 400 responses. The problem, as far as I can see, is that we only have the email or phone number that the person entered -- by the time we're sending you the request we have no way to go back to them to double check. We do have some validation in our form already, which should prevent them from entering invalid data here, but what would you like us to do on our end after getting a 400 response from you? That is, I don't think there's any way for us to fix the data and re-send. We're going to be sending you all the data we have to begin with. Would allReady just not service this request?

business validation

The endpoint I pointed you to earlier is only intended to update the status of an existing request, which is why it only takes the status (e.g. completed, inprogress) and acceptance (a boolean indicating whether or not allReady will service the request). This will work with some of your examples but not others.

  • inability to service the request b/c of region/zip

This is what the status and acceptance information was originally intended to handle. Here you'd pass us back status: new, acceptance: false and we'd continue to manage the request on our side, knowing that you weren't tracking it. :+1:

  • duplicate record sent to us with matching serial in our db with a status of new (this probably won't happen, but we have to check)

I would expect the value of acceptance to be false here. I don't love the idea of manually checking the value of status for string values like "duplicate," but that's one option. Assuming we do send you a duplicate record, something has definitely gone wrong and we'll have to debug rather than just having allReady quietly decide not to accept the second copy. @kfogel, ideas about how to manage violations of business logic like this? That is, we might need to add more fields to our endpoint so that you (@mgmccarthy) can send us back more details along with acceptance: false.

mgmccarthy commented 8 years ago

@cecilia-donnelly, excellent summary. This is a good conversation that's getting us somewhere.

field validations

what would you like us to do on our end after getting a 400 response from you? That is, I don't think there's any way for us to fix the data and re-send. We're going to be sending you all the data we have to begin with. Would allReady just not service this request?

As far as the field validations are concerned, you are correct, there is not too much you can do at that point. Until those invalid fields are fixed, you could resend us that Request as many times as you want and it will always return a 400.

In short, it's up to you what you want to do with that Request. Some ideas are finding a way to mark the request as invalid in your system or will marking it as canceled? It's really up to you what compensating actions you would want to take take.

Either way, we would not invoke your API on any 400 status codes we returned to you and the 400 coming back pretty much says we can't service the request.

I can provide additional data back to you in case of a 400 in the style of Google's Json style guide for errors. An example of what would come back could look something like this:

{
    "error":
    {
        "code": 400
        "message": "field validation failed",
        "errors": [
             {"serial":"empty or null"},
             {"Name":"empty or null"},
             {"Email":"not valid email address"}
        ]
    }
}

but again, if there is nothing you can do to fix the 400, then providing this level of detail about why the message failed field validation might not be of any value to you. Just the 400 is enough to know there is something wrong with the request and we cannot service it.

These are the following validations I will run on our end that could result in a 400 returned:

business validations

The endpoint I pointed you to earlier is only intended to update the status of an existing request

This clears up a lot. Thanks so much for explaining it. So, it seems the only time we will invoke your API is when we give you a 202 back and we can either:

and like you said, those return values are outlined in your response directly above as well as @MisterJames comments here

as far as this item:

duplicate record sent to us with matching serial in our db with a status of new (this probably won't happen, but we have to check)

we could invoke your API and give you a new status back, OR we could simply do the check immediately (in process) even though it does require a db hit and then 400 back to you in the case where this request exists with the same serial value in our database and it has a status of new.

That might be a little easier than having to support a new status code in your system, or having us invoke your API for something we can look up when you invoke our endpoint. Let me know what you'd like to do about this item. For now, I'm going to return a 400.

Whew! We're close... a couple more exchanges and we'll be ready to really test this.

Also as an FYI, I mentioned before that @stevejgordon will be taking the code to communicate back to your API, and I will now be taking that code and finishing it up.

Thanks!

mgmccarthy commented 8 years ago

@cecilia-donnelly, what is the actual URL I would use to invoke your API you've outlines here?

@cecilia-donnelly never mind. I found the test url in an email that MisterJames forwarded to me. I'm starting to set things up on this end.

There is one discrepency around the token we pass to you. Here it says to attach the token to the token to the Authorization header key and add it to the http headers of the request.

But in the email I received from MisterJames, it looks like the token should go into the json body of the request?

{
    "acceptance": true,
    "status": "completed",
    "token": "1234"
}

not too sure what the correct way to pass the token to the demo endpoint. If it's easier, you can reach out to me on the AllReady slack channel, which looks like you're a member of.

Thanks!

mgmccarthy commented 8 years ago

@cecilia-donnelly, I am having problems sending a POST to your demo endpoint. I won't put all the details here, but I'm trying to do it using Postman, and the request is timing out.

the error I'm getting back has to do something with a proxy server and the address is an Apache address pointing to the demo server

cecilia-donnelly commented 8 years ago

@mgmccarthy I see the error in the logs (Unhandled rejection TypeError: Cannot read property 'acceptance' of undefined) -- looks like the server can't interpret the body of your request. Can you send me exactly what you're POST-ing? I think I see what's happening has to do with https://github.com/redcross/smoke-alarm-portal/commit/32d39a6231081709ebb034deceb80281c9cf6abe. I've made a small change (see https://github.com/redcross/smoke-alarm-portal/commit/e2c8c82a4ea4d50dea9e217de2083edad32f87ab). Can you try again and let me know if it fixes the problem?

mgmccarthy commented 8 years ago

here is what I've tried posting:

{
    "acceptance": true,
    "status": "completed"
}
{
    "acceptance": true,
    "status": "completed",
    "token": "(not posting the actual value here on GitHub)"
}

for the url, it's the demo test domain (which I'm also not posting on GitHub) with this path: admin/requests/status/[SERIAL]

I'm not too sure if the token value goes into the http headers or in the body of the message

UPDATE

it worked. I posted the first message (w/out the token in the body) and putting the token in the http header

cecilia-donnelly commented 8 years ago

@mgmccarthy exciting! That's great to hear.

mgmccarthy commented 8 years ago

working on integrating the call into our code now and I'll be testing on and off throughout the day.

We're still working on the verification token on our end. We've hit a bit of a snag where tokens that are being issued by our token provider and not being found to be legit when posting data back to our endpoint, and we need to find out why.

cecilia-donnelly commented 8 years ago

@mgmccarthy ah, okay. Thanks for keeping me up to date.

mgmccarthy commented 8 years ago

@cecilia-donnelly, any word on if you want us to report additional information on any 400's returned to you like I outlined in this comment?

mgmccarthy commented 8 years ago

@cecilia-donnelly, so there has finally been some talk and semi-decisions about how we going to "map" incoming Requests into AllReady.

Since we've decided that just assigning the Request to an existing Event with a matching zip code is not realistic b/c of things like zip codes that could cover a lot of geographical area/zip code density issues, we're going to create all the incoming Requests in our system and associate them to an org (in this case, it would be the Red Cross org)...

at that point, we're not too sure whether or not to report back to you if these Requests are serviceable or not, mainly b/c we don't really put the Request "into play" in our system until we associate a Request to an Event. Once that association happens, that Request can then be assigned to an Itinerary, and that's when we can start to take actions to service the Request.

That being said, if we were to call back to your API that all the incoming requests are not serviceable when they first get created in our system (aka, assigned to an organization)... but later, when the Request was associated to an Event, we wanted to invoke your API and say not this Request IS serviceable, does that make sense?

Does your API work that way, or once you receive the initial true/false for serviceable, there is no way to communicate another update back to you?

Thanks

cecilia-donnelly commented 7 years ago

@mgmccarthy Sorry for the radio silence! Answers at last.

any word on if you want us to report additional information on any 400's returned to you like I outlined in this comment?

I think you're right that we don't need the additional detail, since we can't do much with it.

That being said, if we were to call back to your API that all the incoming requests are not serviceable when they first get created in our system (aka, assigned to an organization)... but later, when the Request was associated to an Event, we wanted to invoke your API and say not this Request IS serviceable, does that make sense?

Does your API work that way, or once you receive the initial true/false for serviceable, there is no way to communicate another update back to you?

This makes sense! Our API does allow multiple updates to a given smoke alarm request. So you could definitely respond with a 202 and accepted: false initially, and then later send us another message with accepted: true for that same request id. Works for me.

Thinking...the only problem is if we ever want to allow our users to update the status of requests that you aren't servicing. I know that at least a few regions (e.g. Chicago, ID/MT) are already using their ability to update request status through our interface, but it doesn't look like they're doing so all the time. I would like to allow them to continue doing so if they aren't using allReady, but I don't want to get back into the two masters problem we talked about before. It's partly a user training issue ("In which application do we update request status?"), but I want to minimize the possibility of human error.

mgmccarthy commented 7 years ago

To address this:

the only problem is if we ever want to allow our users to update the status of requests that you aren't servicing. I know that at least a few regions (e.g. Chicago, ID/MT) are already using their ability to update request status through our interface, but it doesn't look like they're doing so all the time

You could add another type of status in your system that would disallow users updating the request via GASA. This status would mean we are "tracking" the Request in our system, but we really can't DO anything with the request until we associate it to an Event.

That being said, there is no guarantee that a Request would ever be associated to an Event in our system (at least not the way it's working at the present moment)... so if users can't update the Request via GASA's UI, and the Request is essentially orphaned in our system b/c it's never associated to an Event, that would be a bad thing (but I don't know if this would actually happen, the Request not being acted on in either system).

One thing that could work is if the Request were to be updated by a user via GASA's UI after we had sent a status of false we would then need to handle updates to our endpoint and essentially remove that Request from our system.

Since we really can't do anything with Requests until they're associated to an Event, deleting the Request at that point would not be a bad thing. Since we update you with true when a Request is associated to an Event, then GASA UI would not be able to update the Request, and we would at that point be actively tracking/doing something with it.

Thoughts?

cecilia-donnelly commented 7 years ago

One thing that could work is if the Request were to be updated by a user via GASA's UI after we had sent a status of false we would then need to handle updates to our endpoint and essentially remove that Request from our system.

Since we really can't do anything with Requests until they're associated to an Event, deleting the Request at that point would not be a bad thing. Since we update you with true when a Request is associated to an Event, then GASA UI would not be able to update the Request, and we would at that point be actively tracking/doing something with it.

Okay, so a few possible workflows:

first

  1. We send you a new Request
  2. You respond with accepted: false
  3. A user updates that request via GASA
  4. We send you some message saying "we're tracking this request now, please delete it"

second

  1. We send you a new Request
  2. You respond with accepted: false
  3. You assign the Request to an Event
  4. You update us with accepted: true
  5. We block users from updating the Request in GASA

third

  1. We send you a new Request
  2. You respond with accepted: true
  3. We block users from updating the Request in GASA

That would work. We'd have to add some code to tell you to delete a given request, for the first workflow. Do we have a sense of how many of the Red Cross regions will be using allReady, @OhMcGoo?