[$1000] Implement new Google Places API

[rafecolton]

Coming from #45432, please implement the new Places API as described in this comment.

We'll follow the usual process of starting with a detailed proposal, but I will assign @rojiphil directly as the Contributor here since he has the most context.

I will be the CME and will also help with getting the API key updated/installed.

---

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015599eafb1cb0d217
• Upwork Job ID: 1819786402974465675
• Last Price Increase: 2024-08-03
• Automatic offers:
• mkhutornyi | Reviewer | 103384006
• rojiphil | Contributor | 103384007

Issue Owner

Current Issue Owner: @rafecolton

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mkhutornyi (External)

[melvin-bot]

Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[rafecolton]

Not sure if the upwork job was created, going to try re-adding the label

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~015599eafb1cb0d217

[melvin-bot]

Current assignee @mkhutornyi is eligible for the External assigner, not assigning anyone new.

[rafecolton]

Per discussion in the linked issue, I'm assigning @rojiphil as the Contributor. @rojiphil, please provide a detailed proposal when you are able for @mkhutornyi to review. Thanks!

[melvin-bot]

📣 @mkhutornyi 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @rojiphil 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[rojiphil]

Thanks @rafecolton for assigning me here. I am looking into this and expect to revert in next couple of days.

[rafecolton]

Any update on when you will be delivering the first proposal @rojiphil?

[rojiphil]

Any update on when you will be delivering the first proposal @rojiphil?

Almost there. I will share the update today.

[rojiphil]

@rafecolton @mkhutornyi A draft PR with the proposed implementation details is available in PR description here. Please have a look. We can take this further based on your feedback.

[rafecolton]

Thanks for the initial writeup. @mkhutornyi please take a look at the proposal and let me know your thoughts.

In order to keep the discussion all in one place, @rojiphil will you please transfer the proposal here? (Feel free to edit your comment above to include the proposal details.)

I have a few initial questions and comments:

1. Are you able to test the current API by making a real API request through our web proxy? If not, how do you plan to test? (i.e. Will you need support from an internal engineer?)
2. Can you please add some detail around what specific changes will need to be made to the BE?
   • The current implementation in the web API essentially proxies the request to https://maps.googleapis.com/maps/api, passing along the query parameters from the front-end after injecting/replacing the api key.
   • Please detail any changes that will be made to the URL, http method, etc. Please ensure this design will remain backwards-compatible. (We either need to add a new proxy endpoint or update the existing one to differentiate between the requests and support both types.)
   • Please also detail what additional scopes need to be added to the API key, or if a new key is required. If it's possible to add scopes to the existing key rather than generate a new one, I think that would be preferred, as it will simplify the BE code a bit.
3. I will be OOO for the rest of the month starting tomorrow, so @Christinadobrzyn can source another engineer to help you test if you are not able to test yourself. That engineer may have to recruit somebody to generate and install the new key or add scopes to the existing one.

[rojiphil]

Proposal

Please re-state the problem that we are trying to solve in this issue.

This PR implements the scope limited to the places migration as used in NewDot.

What is the root cause of that problem?

There is no root cause as this is an improvement.

What changes do you think we should make in order to solve the problem?

Dependencies:

1. Changes in react-native-google-places-autocomplete library which is implemented in this PR
2. API Key updation for Places API (New) in Google Cloud
3. BE changes to support API changes for Autocomplete (New) and Place Details(New)

The implementation details for the relevant steps as mentioned here are as follows:

Let us introduce a isNewPlacesAPI property in GooglePlacesAutocomplete component here to introduce support for Places (New) api. By default, we can set this to false here so that the current implementation of Places api is used by default. This way, we do not break existing systems using the old Places API. Further, to make use of the Places API (New), the FE can send isNewPlacesAPI here.

1. Use HTTP POST instead of HTTP GET for Autocomplete (New) API uses as mentioned here.

For Autocomplete (New) request, we can send a HTTP POST request here and also set the new url for POST i.e. ${url}/v1/places:autocomplete. Further, we can send the request body with the supported parameters here.
In addition, we also need to update the API for Place Details (New) with url ${url}/v1/places/${rowData.place_id} here

2. Handle the format changes in response structure for Autocomplete (New) API as mentioned here

Format changes include changing the request parameters to languageCode, includedRegionCodes and locationBias here. For the response though, we need to filter the results and handle the format changes. This is done here and here

3. Use the required parameter fieldmask for Place Details (New) API as mentioned here. This way we can control the billing by including what we need in field mask

Let us introduce a fields property in GooglePlacesAutocomplete component here to specify the desired response fields. By default, we can set this to * here so that all the fields are sent back. In our case, we set fields to make use of Location Only (Place Details) SKU thereby reducing our API cost. The specific value of fields is set here.

4. Use session tokens to group autocomplete and place details which will help in reduced billing. This is referenced here.

To use sessions, we can create a sessionToken state here and use uuid library to generate v4 uuids for the session. This session token will be used in every Autocomplete (New) API here. And the session token will be reset to a new uuid here after using it for the last time in the Place Details (New) API here

5. Handle the format changes related to Place object for in response structure of Place Details (New) API as referenced here

Since the response has changed Place Details (New) API, there will not be any status. Hence, we can depend on id here and pass the details to the onPress handler. The FE saveLocationDetails is modified to support the format changes in the Place Details (New) API response.

What alternative solutions did you explore? (Optional)

[rojiphil]

• Are you able to test the current API by making a real API request through our web proxy? If not, how do you plan to test? (i.e. Will you need support from an internal engineer?)

We will need the support of an internal engineer to make the BE changes.
With the current changes, the BE returns 666 code response. So, to test until we are BE ready, I have injected some test data in the code which will be removed once the BE is ready. As soon as the BE is ready, the FE should work seamlessly though.

• Can you please add some detail around what specific changes will need to be made to the BE?

  • The current implementation in the web API essentially proxies the request to https://maps.googleapis.com/maps/api, passing along the query parameters from the front-end after injecting/replacing the api key.
  • Please detail any changes that will be made to the URL, http method, etc. Please ensure this design will remain backwards-compatible. (We either need to add a new proxy endpoint or update the existing one to differentiate between the requests and support both types.)

The specific changes are as follows:

1. Current implementation proxies the request to https://maps.googleapis.com/maps/api. The new implementation should proxy the request to https://places.googleapis.com for both Autocomplete API (New) and Place Details (New). And, as usual, we need to pass along the query parameters from the FE after injecting/replacing the api key.

2. I proposed to add new proxy endpoints. That way, our design would be backward-compatible.

For Autocomplete API (New):
HTTP Method: POST
Query URL begins with /v1/places:autocomplete

For Place Details API (New):
HTTP Method: GET
Query URL begins with /v1/places/${rowData.place_id} where rowData.place_id represents the placeId of the place for which details are to be fetched.

• Please also detail what additional scopes need to be added to the API key, or if a new key is required. If it's possible to add scopes to the existing key rather than generate a new one, I think that would be preferred, as it will simplify the BE code a bit.

The steps required to enable Places API (New) to the API key is mentioned here. I think we can update the existing API key itself if it is going to simplify the BE.

[rafecolton]

Looking at this now

[rafecolton]

Marking Reviewing as all related PRs are under review currently

[Christinadobrzyn]

With Reviewing, should we move this from daily to weekly?

[rafecolton]

Let's keep it at Daily. In fact, I'll take it out of reviewing and make @rojiphil the issue owner until the final App PR is ready to review.

[rafecolton]

Final PR is now up for review! Cool to see that Melvin added the labels automagically ✨

[rafecolton]

PR is failing on android without content-type header, but with the header, it produces a CORS error. I dug into that before when I was testing and it was a rabbit hole, but since it seems to be required here, I'll have to dig more. Setting myself as the issue owner now since this is blocked on me. Might not be able to get back to this until next week, have some higher priority issues assigned currently

[rafecolton]

Thanks for your patience on this @rojiphil - I still plan on getting back to this as soon as I wrap up some other issues

[melvin-bot]

This issue has not been updated in over 15 days. @rafecolton, @rojiphil, @Christinadobrzyn, @mkhutornyi eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[rafecolton]

Sorry to leave this sit for so long. It's still on my radar, just not able to prioritize at the moment.

[melvin-bot]

@rafecolton, @rojiphil, @Christinadobrzyn, @mkhutornyi, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

[rojiphil]

@rafecolton Oh! Melvin closed this issue. Can you please reopen this? Or is this not a priority anymore?

[rafecolton]

This has never been a super high priority per se, but I would love to get it implemented because it will save us some money and pay for itself. I'm currently looking for a volunteer to take over the backend investigation and get you unblocked there.

[rafecolton]

Laying out the tl;dr of the problem:

We are switching to a new Google API endpoint for maps that uses POST instead of GET. It was working on most platforms but broke on Android because Android required we send the Content-Type: application/json header. However when we send that header to web / the VM, the request does not go through, and I wasn't able to figure out why. I don't know if it's a problem with the WAF, Web code, _before.php stuff, or the nginx config. It is only reproducible in the VM - when sending the request directly to Google and not through Web, it works just fine.

@rojiphil anything I'm missing or any color you can add?

[rojiphil]

@rafecolton The problem statement looks fine to me but what baffles me is that the FE request with the Content-type header succeeds for native(Android/iOS) and Web platforms (i.e. via Chrome/Safari browser) but fails for the Desktop platform with CORs violation as mentioned here. Does the FE request go through different paths in BE based on the type of FE platform (i.e. Web/Native/Desktop)?

[rafecolton]

They go through the same path on the actual app/site, but there are some differences in dev

[rojiphil]

Ok. We are almost there to get this PR done except for this nasty issue. Hoping that we break the deadlock soon.

[rafecolton]

Ideally I'd like to hand this off to a backend external contributor, but they are still ramping up. @flodnv any thoughts?

I'm also curious if we could add a switch to enable the new feature everywhere except android. That would enable us to close this issue out and get you paid, then we could come back and update android when it's a little higher priority. @tgolen what do you think about that?

[flodnv]

Ideally I'd like to hand this off to a backend external contributor, but they are still ramping up. @flodnv any thoughts?

Same as here 😄