Pocket / curation-tools-frontend

DEPRECATED
Mozilla Public License 2.0
10 stars 52 forks source link

API Spec #112

Open jpetto opened 3 years ago

jpetto commented 3 years ago

Adding a spec for the API. Currently WIP

nina-py commented 3 years ago

@jpetto, @kkyeboah, just adding a note here that Apollo can connect to a REST API as well with the apollo-link-rest package: https://www.apollographql.com/docs/link/links/rest/. The queries would have to be adjusted but overall I think this would be quick and painless to migrate to on the frontend instead of moving wholesale to Axios & Redux. As you can probably tell I am very keen to keep Apollo! :)

nina-py commented 3 years ago

Another question: where should the link "Learn how to write alt-text" in the Edit & Approve form go? It's currently hardcoded to getpocket.com.

screenshot

nina-py commented 3 years ago

On the same form, there is a "Topic" field. From memory, the MVP is supposed to have a list of pre-defined topics there. Is there a list of those topics that we could use in the form? And with regards to the API, will this list of topics be stored in another table? The topic that I get from the API will likely be a string that I can display on the frontend, but what will the Edit & Approve form need to post? The string representation of the topic, i.e. "History", or some sort of ID?

nina-py commented 3 years ago

Some questions about the UI, specifically about UI updates around API calls from forms. I feel that the spinner that I show on load works well when you're waiting for a list of prospects to load, but when it comes to submitting form data it's not as straightforward.

At the moment, on the Add Story page, I show a spinner and any resulting API errors on top of the form, pushing down the form itself: add-story

The Edit & Approve page has a longer form, so if you click on any of the buttons at the bottom of the form, you won't see this spinner if I place it on top of the page. For the time being, I add it to the bottom of the page, after the buttons:

Screenshot from 2021-01-20 11-48-05

One solution would be to use modals and overlay the spinner over the form. After digging through the Figma comps for any hints, I found this on the "Third round" screen:

figma

Is that something I could/should use instead of the spinner? That mockup appears to show that on submit I need to hide the action buttons and show a message on the screen while the API request is in progress.

nina-py commented 3 years ago

Additionally, once a story has been approved successfully, I'm not redirecting users anywhere just yet. Instead, I show a toast message to indicate success:

toast

I added this as a temporary thing to snow something to the user instead of silently updating the prospect, but now I'm thinking perhaps API errors should be displayed this way as well instead of being rendered within the page? So that they don't push down any other content on the page, are always visible and easy to dismiss. Let me know what you think.

nina-py commented 3 years ago

Hi @jpetto, I was looking at the "Frontend questions" doc to make sure that I've got the ordering of data on the Prospects tabs right, and came across this:

  • What happens when a prospect in the queue is 'Snoozed'? It just stays in the SNOOZED tab until someone acts on it. No need to put it back into PROSPECTS after a certain period of time.

This seems to suggest that the snoozedUntil field is not needed, and perhaps a SNOOZED state should be used instead for simplicity.

nina-py commented 3 years ago

Here is what the frontend currently expects from the API that is not (yet?) reflected in the draft PR:

  1. The snoozedUntil field is gone and is not used anywhere.
  2. There's a new removalReason (string|null) field on the Prospect type.
  3. When making changes to prospects, the updatedAt field is not set to the current date and time - the frontend expects this will be handled by the backend. Is this expectaction correct or should the frontend be setting this field with each mutation it sends to the API?
nina-py commented 3 years ago

Additionally, I don't think I can send a prioritize value with the API payload if it doesn't correspond to a field on a type in a GraphQL schema.