search

ShelterTechSF / askdarcel-web

The "Ask Darcel" web app.
GNU General Public License v3.0
28 stars 22 forks source link

Hook up bookmarks APIs. #1366

Closed richardxia closed 5 months ago

richardxia commented 5 months ago

Refs #1349.

This replaces all the placeholder data related to bookmarks with actual API calls to the newly added Bookmarks and Folders APIs. There are a few mismatches between the existing frontend code and the backend APIs that we actually ended up implementing. For expediency, we mostly just work around gaps or omissions from the backend APIs, but ideally these should be addressed in the future.

Two recurring issues are that the Bookmarks API does not have a name field, and it does not return the updated_at field. The former doesn't even exist in the database table definition, but the latter is just an omission. Some of the changes in this commit work around the lack of these fields, including making an API request to the bookmarked service or resource to grab its name.

There are some awkward places where we make a large number of API calls to fetch information that isn't easily accessible, such as getting the largest bookmark order number, which we can only do by querying all bookmarks owned by the user.

Finally, there is some awkwardness due to the fact that the user's ID is not easily accessible in the frontend code. The JWT from Auth0 only contains the "external ID" coming from Auth0, and our authentication code is too complicated to easily slip in our API's user IDs into the same React context, mostly due to needing to correctly handle the case where a user is signing up or simply logging in. We opt for the safer, though awkward strategy of always querying the API for the user's ID just before making any API call that actually requires the user ID. This ensures that we will always have access to the user ID, at the cost of requiring an additional API request for every actual API request we really want to make.

Also, I really want to thank @schroerbrian for writing most of the frontend code for this feature in the first place! I really didn't have to do too much work besides replacing the placeholder data with actual calls to the real APIs, and most of the complexity of this PR is due to (somewhat expected) mismatches between what the APIs provide and what the frontend needs. Hopefully we can address these mismatches in the future.

sentry-io[bot] commented 5 months ago

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

Did you find this useful? React with a 👍 or 👎

richardxia commented 5 months ago

Whoa, that's a cool Sentry-GitHub integration. Yes, that bug is caused by this PR, and it's because I'm intentionally not handling the case where bookmarks are created outside of folders. The UI doesn't actually allow you to even create bookmarks outside of folders for now, and the only reason this came in staging is because @jjfreund and I had manually made curl requests to the API for testing purposes. I can fix this up by manually editing the DB entries to stick this inside of a folder.