Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEATURE REQ: Auto-subscribe to Healthcare Authorities #425

Closed
penrods opened this issue Apr 10, 2020 · 16 comments
Closed

FEATURE REQ: Auto-subscribe to Healthcare Authorities #425

penrods opened this issue Apr 10, 2020 · 16 comments
Assignees

Comments

@penrods
Copy link
Contributor

penrods commented Apr 10, 2020

Currently the user must manually subscribe to a Healthcare Authority. This will grow to a point where it is a difficult UI to work with. I propose the following:

  1. Extend the Healthcare Authority yaml which lives on the safe-places repo to include a GPS bounding box.
  2. If the user has no existing Healthcare Authority, the app checks that file once every 24 hours. If the user's current GPS coordinate is within any bounding box, they should be given a popup notification that a Healthcare Authority exists in their are, "Would you like to subscribe?"
    ** This should only happen until they have the first authority to avoid overloading the server with millions of requests **
  3. A similar process can be used when the user manually enters the Subscription page. I'm not worried about server overload on a manual process. This can limit the list to authorities which contain any of the points in their full 28-day GPS history, then have a checkbox that will show All Authorities.

The list which they are checked against should be a "production" URL from http://raw.githack.com/. This will provide decent buffering of the Github server infrastructure, but it is VERY IMPORTANT we purge the cache anytime a new Healthcare Authority is added to the YAML.

This is the production (cached for a year) CDN URL:
https://rawcdn.githack.com/tripleblindmarket/safe-places/940edc081b8a4771deca806ed570eda907b5a261/healthcare-authorities.yaml

@tstirrat
Copy link
Contributor

@Patrick-Erichsen was looking into this, I think

@Patrick-Erichsen
Copy link
Contributor

I believe #375 is the same body of work. But this explanation is much more in-depth, so we could close #375 in favor of this.

@Patrick-Erichsen
Copy link
Contributor

Patrick-Erichsen commented Apr 11, 2020

@penrods Is there a proposed schema for what the HCA YAML file might look like? The current one does not include the GPS bounding box required to complete this issue.

@Patrick-Erichsen
Copy link
Contributor

Patrick-Erichsen commented Apr 16, 2020

@penrods @tstirrat any updates on a proposed schema for the GPS bounding box of a Healthcare Authority?

I'm planning on implementing an isPointInRegion() function that can be used to check if a user's GPS history has any points that are within the bounding box of an HCA, but without the schema for that bounding box, I can't get started.

The current mock YAML here has this schema:

- Test Authority:
  - {url: "https://raw.githack.com/tripleblindmarket/safe-places/develop/examples/safe-paths.json"}

Other than that I think I've made some good progress. I pulled out some logic in LocationTracking and ChooseProvider into a common HCAService file with some tests here, and the screenshot below is the UI to filter HCA's by a user's GPS history.

Screen Shot 2020-04-15 at 11 15 31 PM

Once I have that isPointInRegion() implemented, this should be close to being ready for review.

@tremblerz
Copy link
Contributor

Do you think we should just generate Geohashes on both sides?

@penrods
Copy link
Contributor Author

penrods commented Apr 16, 2020

@tremblerz, this is really a bounding box so keeping it in distinct lat/lon is useful. Pseudo-code is:

inside = lat < latMax && lat > latMin && lon < lonMax && lon > lonMin

I'm proposing the following YAML format:

Authorities:
- Mairie de PAP/MSSP:
  - {url: "https://vault.tripleblind.app/safe_path/5673742378205184/"} 
  - bounds: { "ne": { "lat": 42.42025904738132, "lon": -70.93670068664551 }, "sw": { "lat":42.29988330010084, "lon": -71.2516993133545} }

I don't love the length of the "bounds" line, but that is easily copy/pasted, as opposed to:

Authorities:
- Mairie de PAP/MSSP:
  - {url: "https://vault.tripleblind.app/safe_path/5673742378205184/"} 
  - bounds: 
     ne: { "lat": 42.42025904738132, "lon": -70.93670068664551 }
     sw: { "lat":42.29988330010084, "lon": -71.2516993133545}

Which has the same structure, just formatted differently visually.

@Patrick-Erichsen
Copy link
Contributor

I think that the bounds are essentially just a single piece of info as well, so it makes sense to keep it as a single field. It would be weird to break out the URL above into something like

protocol: ...
hostname: ... 
resource: ...

@Patrick-Erichsen
Copy link
Contributor

I noticed that in our async storage for the LOCATION_DATA key, the location objects use the full words latitude and longitude, e.g.

{ "latitude": 42.42025904738132, "longitude": -70.93670068664551, "time": 12345 }

I'd like to propose we use the same naming convention for the bounding box schema as well.

@penrods
Copy link
Contributor Author

penrods commented Apr 17, 2020

Fine with me, consistency is a good thing -- although I prefer the shorter version and which I'd taken the time to convert it back in the day of a month ago. :)

@Patrick-Erichsen
Copy link
Contributor

Patrick-Erichsen commented Apr 18, 2020

So for the second bullet:

If the user has no existing Healthcare Authority, the app checks that file once every 24 hours. If the user's current GPS coordinate is within any bounding box, they should be given a popup notification that a Healthcare Authority exists in their are, "Would you like to subscribe?"
** This should only happen until they have the first authority to avoid overloading the server with millions of requests **

The way I was going to implement that was through an Alert that would use the react-navigate navigation prop to direct a user to the ChooseProvider screen.

However, the checkIntersect() function, which is the method that runs every 12 hours and should perform the check described above, does not have access to that prop.

We call checkIntersect() from LocationTracking.js, which does have access to that prop, but would require a bit of prop-drilling.

I can see this being a problem in other places in the codebase. What are people's thoughts around pulling in the useNavigation() hook from react-navigation? This would allow us to access the navigation prop from any component.


Update
Based on feedback from @tremblerz, I am going to pull the findNewAuthorities() logic out of checkIntersect() and put it back into LocationTracking.js. This will prevent us from needing to pull in the useNavigation() hook since LocationTracking.js has the navigation prop we need.

@Patrick-Erichsen
Copy link
Contributor

Patrick-Erichsen commented Apr 18, 2020

So I have a couple of UI/UX questions for point 2 of this ticket:

  • Is it possible that there could be overlapping bounds for two health care authorities? If that is the case, how should we go about "auto-subscribing"? I was thinking that rather than auto subscribing, in general, use an alert to redirect the user to the "ChooseProvider", where they can then manually subscribe.
  • Should the "popup" be a push notification? Or just an in-app Alert?
  • For the 24 hour check, how should we go about de-coupling that from the checkIntersect() function (currently the only function that executes from BackgroundTaskServices.start())? Maybe just create another static method on that class? This implementation will be dependent on whether or not we want the "popup" to be in-app or push. If we do want it to just be in-app, how do we go about alerting the user? Just give them the notification immediately the next time that they open the app?

@tstirrat
Copy link
Contributor

@kenpugsley is working on a refactor of intersections code in #529 so check with him on that.

re UX, I'd say for now, an in app notify. @tremblerz what do you think?

@penrods
Copy link
Contributor Author

penrods commented Apr 19, 2020

I think you should continue to poll even if the user has their first authority. Here are several use cases:

  • The user is subscribed to their home city authority. They visit a new city. They should be prompted to subscribe to new city.
  • A city, such as Kansas City, straddles multiple healthcare authorities -- county/state lines dictate jurisdictions. The bounding box of the counties can encompass the entire city, and residents should be prompted as all of the authorities come online.

Hitting this once a day should be fine. I've talked to the raw.githack.com team and they are prepared for this traffic.

@Patrick-Erichsen
Copy link
Contributor

Patrick-Erichsen commented Apr 19, 2020

Okay, so I can update the 12-hour check to trigger an in-app notify anytime that the user is in the bounds of an HCA they haven't subscribed to yet. I think the best way to do that is to add another task to BackgroundTaskServices.

I don't believe we can schedule more than one BackgroundFetch callback, so I think this will need to run at the same 12-hour frequency that we configure for checkIntersect(). There is a new scheduleTask feature, but it sounds like there is an iOS bug that would be a blocker for us.

Will a 12-hour frequency be an issue for the raw.githack folks?

@penrods to your point about there being overlapping jurisdictions, I think it makes sense to just direct the user to the ChooseProvider screen instead of "auto-subscribing". If there were two or more HCA's in the user's current region, there isn't a good way to let them choose their subscription in an alert.

@penrods
Copy link
Contributor Author

penrods commented Apr 20, 2020

I think githack can support once every 12 hours for quite a while. If it gets to be too much, we can easily add logic to save state and only do it every-other time.

As for the Choose Provider screen, I think just bringing it up from the Notification would be fine. I do believe we will need to implement a default filter by region very quickly on the Add button. Picking from 2 isn't bad, but picking from dozens will be confusing. I think a simple element like a checkbox next to the Add button with "[ X ] Only show Healthcare Authorities in my area" would do the trick.

That work can definitely be done incrementally -- the first pass could just create an alert and bring up that screen.

@tstirrat
Copy link
Contributor

tstirrat commented May 2, 2020

This is waiting on some UX updates. We're tracking it in Jira https://pathcheck.atlassian.net/browse/SAF-40

@tstirrat tstirrat closed this as completed May 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants