Skip to content
This repository was archived by the owner on Apr 22, 2021. It is now read-only.

API Spec #112

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

API Spec #112

wants to merge 6 commits into from

Conversation

jpetto
Copy link

@jpetto jpetto commented Jan 14, 2021

Adding a spec for the API. Currently WIP

@nina-py
Copy link
Contributor

nina-py commented Jan 15, 2021

@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
Copy link
Contributor

nina-py commented Jan 15, 2021

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
Copy link
Contributor

nina-py commented Jan 15, 2021

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
Copy link
Contributor

nina-py commented Jan 20, 2021

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
Copy link
Contributor

nina-py commented Jan 20, 2021

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
Copy link
Contributor

nina-py commented Feb 7, 2021

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
Copy link
Contributor

nina-py commented Mar 8, 2021

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
Copy link
Contributor

nina-py commented Mar 8, 2021

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants