Merge pull request #1929 from omnivore-app/fix/save-request

Revert web changes and make id, url optional for save request

packages/api/src/elastic/pages.ts

@@ -335,13 +335,15 @@ export const getPageByParam = async <K extends keyof ParamSet>(
     const params = {
       query: {
         bool: {
-          filter: Object.keys(param).map((key) => {
-            return {
+          filter: Object.keys(param)
+            .filter(
+              (key) => param[key as K] !== undefined && param[key as K] !== null
+            ) // filter out undefined and null values
+            .map((key) => ({
               term: {
                 [key]: param[key as K],
               },
-            }
-          }),
+            })),
         },
       },
       size: 1,

packages/api/src/generated/graphql.ts

@@ -1752,7 +1752,8 @@ export type QueryArticleArgs = {
 
 
 export type QueryArticleSavingRequestArgs = {
-  url: Scalars['String'];
+  id?: InputMaybe<Scalars['ID']>;
+  url?: InputMaybe<Scalars['String']>;
 };
 
 
@@ -5079,7 +5080,7 @@ export type ProfileResolvers<ContextType = ResolverContext, ParentType extends R
 export type QueryResolvers<ContextType = ResolverContext, ParentType extends ResolversParentTypes['Query'] = ResolversParentTypes['Query']> = {
   apiKeys?: Resolver<ResolversTypes['ApiKeysResult'], ParentType, ContextType>;
   article?: Resolver<ResolversTypes['ArticleResult'], ParentType, ContextType, RequireFields<QueryArticleArgs, 'slug' | 'username'>>;
-  articleSavingRequest?: Resolver<ResolversTypes['ArticleSavingRequestResult'], ParentType, ContextType, RequireFields<QueryArticleSavingRequestArgs, 'url'>>;
+  articleSavingRequest?: Resolver<ResolversTypes['ArticleSavingRequestResult'], ParentType, ContextType, Partial<QueryArticleSavingRequestArgs>>;
   articles?: Resolver<ResolversTypes['ArticlesResult'], ParentType, ContextType, Partial<QueryArticlesArgs>>;
   deviceTokens?: Resolver<ResolversTypes['DeviceTokensResult'], ParentType, ContextType>;
   feedArticles?: Resolver<ResolversTypes['FeedArticlesResult'], ParentType, ContextType, Partial<QueryFeedArticlesArgs>>;

packages/api/src/generated/schema.graphql

@@ -1246,7 +1246,7 @@ type Profile {
 type Query {
   apiKeys: ApiKeysResult!
   article(format: String, slug: String!, username: String!): ArticleResult!
-  articleSavingRequest(url: String!): ArticleSavingRequestResult!
+  articleSavingRequest(id: ID, url: String): ArticleSavingRequestResult!
   articles(after: String, first: Int, includePending: Boolean, query: String, sharedOnly: Boolean, sort: SortParams): ArticlesResult!
   deviceTokens: DeviceTokensResult!
   feedArticles(after: String, first: Int, sharedByUser: ID, sort: SortParams): FeedArticlesResult!

packages/api/src/resolvers/article_saving_request/index.ts

@@ -56,8 +56,13 @@ export const articleSavingRequestResolver = authorized<
   ArticleSavingRequestSuccess,
   ArticleSavingRequestError,
   QueryArticleSavingRequestArgs
->(async (_, { url }, { models, claims }) => {
-  const page = await getPageByParam({ url, userId: claims.uid })
+>(async (_, { id, url }, { models, claims }) => {
+  const params = {
+    _id: id || undefined,
+    url: url || undefined,
+    userId: claims.uid,
+  }
+  const page = await getPageByParam(params)
   if (!page) {
     return { errorCodes: [ArticleSavingRequestErrorCode.NotFound] }
   }

packages/api/src/schema.ts

@@ -2526,7 +2526,7 @@ const schema = gql`
     getFollowers(userId: ID): GetFollowersResult!
     getFollowing(userId: ID): GetFollowingResult!
     getUserPersonalization: GetUserPersonalizationResult!
-    articleSavingRequest(url: String!): ArticleSavingRequestResult!
+    articleSavingRequest(id: ID, url: String): ArticleSavingRequestResult!
     newsletterEmails: NewsletterEmailsResult!
     reminder(linkId: ID!): ReminderResult!
     labels: LabelsResult!

packages/api/test/resolvers/article_saving_request.test.ts

@@ -16,9 +16,17 @@ import * as createTask from '../../src/utils/createTask'
 import { createTestUser, deleteTestUser } from '../db'
 import { graphqlRequest, request } from '../util'
 
-const articleSavingRequestQuery = (url: string) => `
+const articleSavingRequestQuery = ({
+  id,
+  url,
+}: {
+  id?: string
+  url?: string
+}) => `
   query {
-    articleSavingRequest(url: "${url}") {
+    articleSavingRequest(id: ${id ? `"${id}"` : null}, url: ${
+  url ? `"${url}"` : null
+}) {
       ... on ArticleSavingRequestSuccess {
         articleSavingRequest {
           id
@@ -117,19 +125,32 @@ describe('ArticleSavingRequest API', () => {
 
   describe('articleSavingRequest', () => {
     let url: string
+    let id: string
 
     before(async () => {
       url = 'https://blog.omnivore.app/2'
       // create article saving request
-      await graphqlRequest(
+      const res = await graphqlRequest(
         createArticleSavingRequestMutation(url),
         authToken
       ).expect(200)
+      id = res.body.data.createArticleSavingRequest.articleSavingRequest.id
     })
 
     it('returns the article saving request if exists', async () => {
       const res = await graphqlRequest(
-        articleSavingRequestQuery(url),
+        articleSavingRequestQuery({ url }),
+        authToken
+      ).expect(200)
+
+      expect(
+        res.body.data.articleSavingRequest.articleSavingRequest.status
+      ).to.eql(ArticleSavingRequestStatus.Processing)
+    })
+
+    it('returns the article saving request by id', async () => {
+      const res = await graphqlRequest(
+        articleSavingRequestQuery({ id }),
         authToken
       ).expect(200)
 
@@ -140,7 +161,7 @@ describe('ArticleSavingRequest API', () => {
 
     it('returns not_found if not exists', async () => {
       const res = await graphqlRequest(
-        articleSavingRequestQuery('invalid-id'),
+        articleSavingRequestQuery({ id: 'invalid-id' }),
         authToken
       ).expect(200)
 

packages/web/components/templates/homeFeed/AddLinkModal.tsx

@@ -1,14 +1,17 @@
-import { useCallback, useState } from 'react'
-import toast from 'react-hot-toast'
-import { saveUrlMutation } from '../../../lib/networking/mutations/saveUrlMutation'
-import { showErrorToast } from '../../../lib/toastHelpers'
+import {
+  ModalRoot,
+  ModalContent,
+  ModalOverlay,
+  ModalTitleBar,
+  ModalButtonBar,
+} from '../../elements/ModalPrimitives'
+import { VStack, Box } from '../../elements/LayoutPrimitives'
 import { Button } from '../../elements/Button'
 import { FormInput } from '../../elements/FormElements'
-import { Box, VStack } from '../../elements/LayoutPrimitives'
-import {
-  ModalButtonBar, ModalContent,
-  ModalOverlay, ModalRoot, ModalTitleBar
-} from '../../elements/ModalPrimitives'
+import { useState, useCallback } from 'react'
+import { saveUrlMutation } from '../../../lib/networking/mutations/saveUrlMutation'
+import toast from 'react-hot-toast'
+import { showErrorToast } from '../../../lib/toastHelpers'
 
 type AddLinkModalProps = {
   onOpenChange: (open: boolean) => void
@@ -21,7 +24,7 @@ export function AddLinkModal(props: AddLinkModalProps): JSX.Element {
     async (link: string) => {
       const result = await saveUrlMutation(link)
       // const result = await saveUrlMutation(link)
-      if (result) {
+      if (result && result.jobId) {
         // eslint-disable-next-line @typescript-eslint/no-unused-vars
         toast(
           () => (
@@ -32,7 +35,7 @@ export function AddLinkModal(props: AddLinkModalProps): JSX.Element {
                 style="ctaDarkYellow"
                 autoFocus
                 onClick={() => {
-                  window.location.href = `/article/sr/${encodeURIComponent(link)}` // encode url
+                  window.location.href = `/article/sr/${result.jobId}`
                 }}
               >
                 Read Now

packages/web/components/templates/homeFeed/HomeFeedContainer.tsx

@@ -1,51 +1,51 @@
-import * as Progress from '@radix-ui/react-progress'
-import axios from 'axios'
-import { Action, createAction, useKBar, useRegisterActions } from 'kbar'
-import debounce from 'lodash/debounce'
-import { useRouter } from 'next/router'
-import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
+import { Box, HStack, VStack } from './../../elements/LayoutPrimitives'
 import Dropzone from 'react-dropzone'
-import { Toaster } from 'react-hot-toast'
-import TopBarProgress from 'react-topbar-progress-indicator'
-import { useFetchMore } from '../../../lib/hooks/useFetchMoreScroll'
-import { usePersistedState } from '../../../lib/hooks/usePersistedState'
-import { libraryListCommands } from '../../../lib/keyboardShortcuts/navigationShortcuts'
-import { useKeyboardShortcuts } from '../../../lib/keyboardShortcuts/useKeyboardShortcuts'
-import {
-  PageType,
-  State
-} from '../../../lib/networking/fragments/articleFragment'
-import { Label } from '../../../lib/networking/fragments/labelFragment'
-import { setLabelsMutation } from '../../../lib/networking/mutations/setLabelsMutation'
-import { uploadFileRequestMutation } from '../../../lib/networking/mutations/uploadFileMutation'
-import {
-  SearchItem,
-  TypeaheadSearchItemsData,
-  typeaheadSearchQuery
-} from '../../../lib/networking/queries/typeaheadSearch'
+import * as Progress from '@radix-ui/react-progress'
 import type {
   LibraryItem,
-  LibraryItemsQueryInput
+  LibraryItemsQueryInput,
 } from '../../../lib/networking/queries/useGetLibraryItemsQuery'
 import { useGetLibraryItemsQuery } from '../../../lib/networking/queries/useGetLibraryItemsQuery'
 import {
   useGetViewerQuery,
-  UserBasicData
+  UserBasicData,
 } from '../../../lib/networking/queries/useGetViewerQuery'
-import { Button } from '../../elements/Button'
-import { StyledText } from '../../elements/StyledText'
-import { ConfirmationModal } from '../../patterns/ConfirmationModal'
 import { LinkedItemCardAction } from '../../patterns/LibraryCards/CardTypes'
 import { LinkedItemCard } from '../../patterns/LibraryCards/LinkedItemCard'
-import { styled, theme } from '../../tokens/stitches.config'
-import { SetLabelsModal } from '../article/SetLabelsModal'
-import { Box, HStack, VStack } from './../../elements/LayoutPrimitives'
+import { useRouter } from 'next/router'
+import { Button } from '../../elements/Button'
+import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
+import { StyledText } from '../../elements/StyledText'
 import { AddLinkModal } from './AddLinkModal'
-import { EditLibraryItemModal } from './EditItemModals'
+import { styled, theme } from '../../tokens/stitches.config'
+import { libraryListCommands } from '../../../lib/keyboardShortcuts/navigationShortcuts'
+import { useKeyboardShortcuts } from '../../../lib/keyboardShortcuts/useKeyboardShortcuts'
+import { Toaster } from 'react-hot-toast'
+import { useFetchMore } from '../../../lib/hooks/useFetchMoreScroll'
+import { usePersistedState } from '../../../lib/hooks/usePersistedState'
+import { ConfirmationModal } from '../../patterns/ConfirmationModal'
+import { SetLabelsModal } from '../article/SetLabelsModal'
+import { Label } from '../../../lib/networking/fragments/labelFragment'
 import { EmptyLibrary } from './EmptyLibrary'
-import { HighlightItemsLayout } from './HighlightsLayout'
-import { LibraryFilterMenu } from './LibraryFilterMenu'
+import TopBarProgress from 'react-topbar-progress-indicator'
+import {
+  PageType,
+  State,
+} from '../../../lib/networking/fragments/articleFragment'
+import { Action, createAction, useKBar, useRegisterActions } from 'kbar'
+import { EditLibraryItemModal } from './EditItemModals'
+import debounce from 'lodash/debounce'
+import {
+  SearchItem,
+  TypeaheadSearchItemsData,
+  typeaheadSearchQuery,
+} from '../../../lib/networking/queries/typeaheadSearch'
+import axios from 'axios'
+import { uploadFileRequestMutation } from '../../../lib/networking/mutations/uploadFileMutation'
+import { setLabelsMutation } from '../../../lib/networking/mutations/setLabelsMutation'
 import { LibraryHeader } from './LibraryHeader'
+import { LibraryFilterMenu } from './LibraryFilterMenu'
+import { HighlightItemsLayout } from './HighlightsLayout'
 
 export type LayoutType = 'LIST_LAYOUT' | 'GRID_LAYOUT'
 export type LibraryMode = 'reads' | 'highlights'
@@ -288,7 +288,7 @@ export function HomeFeedContainer(): JSX.Element {
         if (username) {
           setActiveCardId(item.node.id)
           if (item.node.state === State.PROCESSING) {
-            router.push(`/${username}/links/${encodeURIComponent(item.node.url)}`)
+            router.push(`/${username}/links/${item.node.id}`)
           } else {
             const dl =
               item.node.pageType === PageType.HIGHLIGHTS

packages/web/lib/networking/queries/useGetArticleSavingStatus.tsx

@@ -6,7 +6,7 @@ import { makeGqlFetcher } from '../networkHelpers'
 import { ArticleAttributes } from './useGetArticleQuery'
 
 type ArticleSavingStatusInput = {
-  url: string
+  id: string
 }
 
 type ArticleSavingStatusResponse = {
@@ -48,11 +48,11 @@ type ArticleSavingStatusError =
   | 'unauthorized'
 
 export function useGetArticleSavingStatus({
-  url,
+  id,
 }: ArticleSavingStatusInput): ArticleSavingStatusResponse {
   const query = gql`
-    query ArticleSavingRequest($url: String!) {
-      articleSavingRequest(url: $url) {
+    query ArticleSavingRequest($id: ID!) {
+      articleSavingRequest(id: $id) {
         ... on ArticleSavingRequestSuccess {
           articleSavingRequest {
             id
@@ -85,7 +85,7 @@ export function useGetArticleSavingStatus({
   `
 
   // poll twice a second
-  const { data, error } = useSWR([query, url], makeGqlFetcher({ url }), {
+  const { data, error } = useSWR([query, id], makeGqlFetcher({ id }), {
     refreshInterval: 500,
   })
 

packages/web/pages/[username]/links/[id].tsx

@@ -1,29 +1,30 @@
 import { useRouter } from 'next/router'
 import { useEffect, useState } from 'react'
-import TopBarProgress from 'react-topbar-progress-indicator'
-import { VStack } from '../../../components/elements/LayoutPrimitives'
-import { ArticleActionsMenu } from '../../../components/templates/article/ArticleActionsMenu'
-import { SkeletonArticleContainer } from '../../../components/templates/article/SkeletonArticleContainer'
+import { useGetArticleSavingStatus } from '../../../lib/networking/queries/useGetArticleSavingStatus'
 import { PrimaryLayout } from '../../../components/templates/PrimaryLayout'
 import {
-  ErrorComponent, Loader
+  Loader,
+  ErrorComponent,
 } from '../../../components/templates/SavingRequest'
+import { ArticleActionsMenu } from '../../../components/templates/article/ArticleActionsMenu'
+import { VStack } from '../../../components/elements/LayoutPrimitives'
 import { theme } from '../../../components/tokens/stitches.config'
-import { useReaderSettings } from '../../../lib/hooks/useReaderSettings'
-import { useGetArticleSavingStatus } from '../../../lib/networking/queries/useGetArticleSavingStatus'
 import { applyStoredTheme } from '../../../lib/themeUpdater'
+import { useReaderSettings } from '../../../lib/hooks/useReaderSettings'
+import { SkeletonArticleContainer } from '../../../components/templates/article/SkeletonArticleContainer'
+import TopBarProgress from 'react-topbar-progress-indicator'
 
 export default function ArticleSavingRequestPage(): JSX.Element {
   const router = useRouter()
   const readerSettings = useReaderSettings()
-  const [url, setUrl] = useState<string | undefined>(undefined)
+  const [articleId, setArticleId] = useState<string | undefined>(undefined)
 
   applyStoredTheme(false)
 
   useEffect(() => {
     if (!router.isReady) return
-    setUrl(router.query.url as string)
-  }, [router.isReady, router.query.url])
+    setArticleId(router.query.id as string)
+  }, [router.isReady, router.query.id])
 
   return (
     <PrimaryLayout
@@ -80,7 +81,7 @@ export default function ArticleSavingRequestPage(): JSX.Element {
           fontSize={readerSettings.fontSize}
           lineHeight={readerSettings.lineHeight}
         >
-          {url ? <PrimaryContent url={url} /> : <Loader />}
+          {articleId ? <PrimaryContent articleId={articleId} /> : <Loader />}
         </SkeletonArticleContainer>
       </VStack>
     </PrimaryLayout>
@@ -88,7 +89,7 @@ export default function ArticleSavingRequestPage(): JSX.Element {
 }
 
 type PrimaryContentProps = {
-  url: string
+  articleId: string
 }
 
 function PrimaryContent(props: PrimaryContentProps): JSX.Element {
@@ -96,7 +97,7 @@ function PrimaryContent(props: PrimaryContentProps): JSX.Element {
   const [timedOut, setTimedOut] = useState(false)
 
   const { successRedirectPath, error } = useGetArticleSavingStatus({
-    url: props.url,
+    id: props.articleId,
   })
 
   useEffect(() => {

packages/web/pages/app/[username]/link-request/[id].tsx

@@ -1,25 +1,25 @@
 import { useRouter } from 'next/router'
 import { useEffect, useState } from 'react'
-import { useSWRConfig } from 'swr'
 import { Box } from '../../../../components/elements/LayoutPrimitives'
-import { PrimaryLayout } from '../../../../components/templates/PrimaryLayout'
-import { ErrorComponent } from '../../../../components/templates/SavingRequest'
-import { cacheArticle } from '../../../../lib/networking/queries/useGetArticleQuery'
 import { useGetArticleSavingStatus } from '../../../../lib/networking/queries/useGetArticleSavingStatus'
+import { ErrorComponent } from '../../../../components/templates/SavingRequest'
+import { useSWRConfig } from 'swr'
+import { cacheArticle } from '../../../../lib/networking/queries/useGetArticleQuery'
+import { PrimaryLayout } from '../../../../components/templates/PrimaryLayout'
 import { applyStoredTheme } from '../../../../lib/themeUpdater'
 
 export default function LinkRequestPage(): JSX.Element {
   applyStoredTheme(false) // false to skip server sync
 
   const router = useRouter()
-  const [url, setUrl] = useState<string | undefined>(undefined)
+  const [requestID, setRequestID] = useState<string | undefined>(undefined)
   const [username, setUsername] = useState<string | undefined>(undefined)
 
   useEffect(() => {
     if (!router.isReady) return
-    setUrl(router.query.url as string)
+    setRequestID(router.query.id as string)
     setUsername(router.query.username as string)
-  }, [router.isReady, router.query.url, router.query.username])
+  }, [router.isReady, router.query.id, router.query.username])
 
   return (
     <PrimaryLayout
@@ -33,8 +33,8 @@ export default function LinkRequestPage(): JSX.Element {
       <Box
         css={{ bg: '$grayBase', height: '100vh', width: '100vw', px: '16px' }}
       >
-        {url && username ? (
-          <PrimaryContent url={url} username={username} />
+        {requestID && username ? (
+          <PrimaryContent requestID={requestID} username={username} />
         ) : (
           <Loader />
         )}
@@ -48,7 +48,7 @@ function Loader(): JSX.Element {
 }
 
 type PrimaryContentProps = {
-  url: string
+  requestID: string
   username: string
 }
 
@@ -58,7 +58,7 @@ function PrimaryContent(props: PrimaryContentProps): JSX.Element {
   const [timedOut, setTimedOut] = useState(false)
 
   const { successRedirectPath, article, error } = useGetArticleSavingStatus({
-    url: props.url,
+    id: props.requestID,
   })
 
   useEffect(() => {

packages/web/pages/article/sr/[id].tsx

@@ -1,29 +1,30 @@
 import { useRouter } from 'next/router'
 import { useEffect, useState } from 'react'
-import TopBarProgress from 'react-topbar-progress-indicator'
-import { VStack } from '../../../components/elements/LayoutPrimitives'
-import { ArticleActionsMenu } from '../../../components/templates/article/ArticleActionsMenu'
-import { SkeletonArticleContainer } from '../../../components/templates/article/SkeletonArticleContainer'
+import { useGetArticleSavingStatus } from '../../../lib/networking/queries/useGetArticleSavingStatus'
 import { PrimaryLayout } from '../../../components/templates/PrimaryLayout'
 import {
-  ErrorComponent, Loader
+  Loader,
+  ErrorComponent,
 } from '../../../components/templates/SavingRequest'
+import { ArticleActionsMenu } from '../../../components/templates/article/ArticleActionsMenu'
+import { VStack } from '../../../components/elements/LayoutPrimitives'
 import { theme } from '../../../components/tokens/stitches.config'
-import { useReaderSettings } from '../../../lib/hooks/useReaderSettings'
-import { useGetArticleSavingStatus } from '../../../lib/networking/queries/useGetArticleSavingStatus'
 import { applyStoredTheme } from '../../../lib/themeUpdater'
+import { useReaderSettings } from '../../../lib/hooks/useReaderSettings'
+import { SkeletonArticleContainer } from '../../../components/templates/article/SkeletonArticleContainer'
+import TopBarProgress from 'react-topbar-progress-indicator'
 
 export default function ArticleSavingRequestPage(): JSX.Element {
   const router = useRouter()
   const readerSettings = useReaderSettings()
-  const [url, setUrl] = useState<string | undefined>(undefined)
+  const [articleId, setArticleId] = useState<string | undefined>(undefined)
 
   applyStoredTheme(false)
 
   useEffect(() => {
     if (!router.isReady) return
-    setUrl(router.query.url as string)
-  }, [router.isReady, router.query.url])
+    setArticleId(router.query.id as string)
+  }, [router.isReady, router.query.id])
 
   return (
     <PrimaryLayout
@@ -77,7 +78,7 @@ export default function ArticleSavingRequestPage(): JSX.Element {
             fontSize={readerSettings.fontSize}
             lineHeight={readerSettings.lineHeight}
           >
-            {url ? <PrimaryContent url={url} /> : <Loader />}
+            {articleId ? <PrimaryContent articleId={articleId} /> : <Loader />}
           </SkeletonArticleContainer>
       </VStack>
     </PrimaryLayout>
@@ -85,7 +86,7 @@ export default function ArticleSavingRequestPage(): JSX.Element {
 }
 
 type PrimaryContentProps = {
-  url: string
+  articleId: string
 }
 
 function PrimaryContent(props: PrimaryContentProps): JSX.Element {
@@ -93,7 +94,7 @@ function PrimaryContent(props: PrimaryContentProps): JSX.Element {
   const [timedOut, setTimedOut] = useState(false)
 
   const { successRedirectPath, error } = useGetArticleSavingStatus({
-    url: props.url,
+    id: props.articleId,
   })
 
   useEffect(() => {