Merge pull request #2942 from omnivore-app/fix/rules

improvement on subscriptions and delete user api

packages/api/src/entity/user.ts

@@ -22,6 +22,7 @@ export enum RegistrationType {
 export enum StatusType {
   Active = 'ACTIVE',
   Pending = 'PENDING',
+  Deleted = 'DELETED',
 }
 
 @Entity()

packages/api/src/repository/user.ts

@@ -1,6 +1,6 @@
 import { In } from 'typeorm'
 import { appDataSource } from '../data_source'
-import { User } from './../entity/user'
+import { StatusType, User } from './../entity/user'
 
 const TOP_USERS = [
   'jacksonh',
@@ -16,7 +16,7 @@ export const MAX_RECORDS_LIMIT = 1000
 
 export const userRepository = appDataSource.getRepository(User).extend({
   findById(id: string) {
-    return this.findOneBy({ id })
+    return this.findOneBy({ id, status: StatusType.Active })
   },
 
   findByEmail(email: string) {

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

@@ -62,11 +62,11 @@ import {
 } from '../../services/labels'
 import {
   createLibraryItem,
-  findLibraryItemById,
   findLibraryItemByUrl,
   findLibraryItemsByPrefix,
   searchLibraryItems,
   updateLibraryItem,
+  updateLibraryItemReadingProgress,
   updateLibraryItems,
 } from '../../services/library_item'
 import { parsedContentToLibraryItem } from '../../services/save_page'
@@ -572,14 +572,8 @@ export const saveArticleReadingProgressResolver = authorized<
         readingProgressTopPercent,
       },
     },
-    { uid, pubsub }
+    { log, pubsub, uid }
   ) => {
-    const libraryItem = await findLibraryItemById(id, uid)
-
-    if (!libraryItem) {
-      return { errorCodes: [SaveArticleReadingProgressErrorCode.NotFound] }
-    }
-
     if (
       readingProgressPercent < 0 ||
       readingProgressPercent > 100 ||
@@ -590,40 +584,26 @@ export const saveArticleReadingProgressResolver = authorized<
     ) {
       return { errorCodes: [SaveArticleReadingProgressErrorCode.BadData] }
     }
-    // If we have a top percent, we only save it if it's greater than the current top percent
-    // or set to zero if the top percent is zero.
-    const readingProgressTopPercentToSave = readingProgressTopPercent
-      ? Math.max(
-          readingProgressTopPercent,
-          libraryItem.readingProgressTopPercent || 0
-        )
-      : readingProgressTopPercent === 0
-      ? 0
-      : undefined
-    // If setting to zero we accept the update, otherwise we require it
-    // be greater than the current reading progress.
-    const updatedPart: QueryDeepPartialEntity<LibraryItem> = {
-      readingProgressBottomPercent:
-        readingProgressPercent === 0
-          ? 0
-          : Math.max(
-              readingProgressPercent,
-              libraryItem.readingProgressBottomPercent
-            ),
-      readingProgressHighestReadAnchor:
-        readingProgressAnchorIndex === 0
-          ? 0
-          : Math.max(
-              readingProgressAnchorIndex || 0,
-              libraryItem.readingProgressHighestReadAnchor
-            ),
-      readingProgressTopPercent: readingProgressTopPercentToSave,
-      readAt: new Date(),
-    }
-    const updatedItem = await updateLibraryItem(id, updatedPart, uid, pubsub)
+    try {
+      const updatedItem = await updateLibraryItemReadingProgress(
+        id,
+        uid,
+        readingProgressPercent,
+        readingProgressTopPercent,
+        readingProgressAnchorIndex,
+        pubsub
+      )
+      if (!updatedItem) {
+        return { errorCodes: [SaveArticleReadingProgressErrorCode.BadData] }
+      }
 
-    return {
-      updatedArticle: libraryItemToArticle(updatedItem),
+      return {
+        updatedArticle: libraryItemToArticle(updatedItem),
+      }
+    } catch (error) {
+      log.error('saveArticleReadingProgressResolver error', error)
+
+      return { errorCodes: [SaveArticleReadingProgressErrorCode.Unauthorized] }
     }
   }
 )

packages/api/src/resolvers/importers/uploadImportFileResolver.ts

@@ -33,14 +33,14 @@ export const uploadImportFileResolver = authorized<
   UploadImportFileSuccess,
   UploadImportFileError,
   MutationUploadImportFileArgs
->(async (_, { type, contentType }, { claims: { uid }, log }) => {
+>(async (_, { type, contentType }, { uid }) => {
   if (!VALID_CONTENT_TYPES.includes(contentType)) {
     return {
       errorCodes: [UploadImportFileErrorCode.BadRequest],
     }
   }
 
-  const user = await userRepository.findOneBy({ id: uid })
+  const user = await userRepository.findById(uid)
   if (!user) {
     return {
       errorCodes: [UploadImportFileErrorCode.Unauthorized],

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

@@ -48,9 +48,7 @@ export const createGroupResolver = authorized<
   MutationCreateGroupArgs
 >(async (_, { input }, { uid, log }) => {
   try {
-    const userData = await userRepository.findOneBy({
-      id: uid,
-    })
+    const userData = await userRepository.findById(uid)
     if (!userData) {
       return {
         errorCodes: [CreateGroupErrorCode.Unauthorized],
@@ -107,9 +105,7 @@ export const createGroupResolver = authorized<
 export const groupsResolver = authorized<GroupsSuccess, GroupsError>(
   async (_, __, { uid, log }) => {
     try {
-      const user = await userRepository.findOneBy({
-        id: uid,
-      })
+      const user = await userRepository.findById(uid)
       if (!user) {
         return {
           errorCodes: [GroupsErrorCode.Unauthorized],

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

@@ -18,9 +18,9 @@ export const savePageResolver = authorized<
   SaveSuccess,
   SaveError,
   MutationSavePageArgs
->(async (_, { input }, ctx) => {
+>(async (_, { input }, { uid }) => {
   analytics.track({
-    userId: ctx.uid,
+    userId: uid,
     event: 'link_saved',
     properties: {
       url: input.url,
@@ -30,9 +30,7 @@ export const savePageResolver = authorized<
     },
   })
 
-  const user = await userRepository.findOneBy({
-    id: ctx.uid,
-  })
+  const user = await userRepository.findById(uid)
   if (!user) {
     return { errorCodes: [SaveErrorCode.Unauthorized] }
   }
@@ -44,11 +42,7 @@ export const saveUrlResolver = authorized<
   SaveSuccess,
   SaveError,
   MutationSaveUrlArgs
->(async (_, { input }, ctx) => {
-  const {
-    claims: { uid },
-  } = ctx
-
+>(async (_, { input }, { uid }) => {
   analytics.track({
     userId: uid,
     event: 'link_saved',
@@ -60,9 +54,7 @@ export const saveUrlResolver = authorized<
     },
   })
 
-  const user = await userRepository.findOneBy({
-    id: uid,
-  })
+  const user = await userRepository.findById(uid)
   if (!user) {
     return { errorCodes: [SaveErrorCode.Unauthorized] }
   }
@@ -74,9 +66,9 @@ export const saveFileResolver = authorized<
   SaveSuccess,
   SaveError,
   MutationSaveFileArgs
->(async (_, { input }, ctx) => {
+>(async (_, { input }, { uid }) => {
   analytics.track({
-    userId: ctx.uid,
+    userId: uid,
     event: 'link_saved',
     properties: {
       url: input.url,
@@ -86,9 +78,7 @@ export const saveFileResolver = authorized<
     },
   })
 
-  const user = await userRepository.findOneBy({
-    id: ctx.uid,
-  })
+  const user = await userRepository.findById(uid)
   if (!user) {
     return { errorCodes: [SaveErrorCode.Unauthorized] }
   }

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

@@ -14,11 +14,9 @@ const INSTALL_INSTRUCTIONS_EMAIL_TEMPLATE_ID =
 export const sendInstallInstructionsResolver = authorized<
   SendInstallInstructionsSuccess,
   SendInstallInstructionsError
->(async (_parent, _args, { claims, log }) => {
+>(async (_parent, _args, { uid, log }) => {
   try {
-    const user = await userRepository.findOneBy({
-      id: claims.uid,
-    })
+    const user = await userRepository.findById(uid)
 
     if (!user) {
       return { errorCodes: [SendInstallInstructionsErrorCode.Unauthorized] }

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

@@ -89,7 +89,8 @@ export const subscriptionsResolver = authorized<
     }
 
     const subscriptions = await queryBuilder
-      .orderBy(`subscription.${sortBy}`, sortOrder, 'NULLS LAST')
+      .orderBy('subscription.status', 'ASC')
+      .addOrderBy(`subscription.${sortBy}`, sortOrder, 'NULLS LAST')
       .getMany()
 
     return {
@@ -184,7 +185,6 @@ export const subscribeResolver = authorized<
         url: input.url || undefined,
         name: input.name || undefined,
         user: { id: uid },
-        status: SubscriptionStatus.Active,
         type: input.subscriptionType || SubscriptionType.Rss, // default to rss
       })
     )

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

@@ -1,5 +1,9 @@
 import * as jwt from 'jsonwebtoken'
-import { RegistrationType, User as UserEntity } from '../../entity/user'
+import {
+  RegistrationType,
+  StatusType,
+  User as UserEntity,
+} from '../../entity/user'
 import { env } from '../../env'
 import {
   DeleteAccountError,
@@ -38,6 +42,7 @@ import {
 import { userRepository } from '../../repository/user'
 import { createUser } from '../../services/create_user'
 import { sendVerificationEmail } from '../../services/send_emails'
+import { updateUser } from '../../services/user'
 import { authorized, userDataToUser } from '../../utils/helpers'
 import { validateUsername } from '../../utils/usernamePolicy'
 import { WithDataSourcesContext } from '../types'
@@ -47,9 +52,7 @@ export const updateUserResolver = authorized<
   UpdateUserError,
   MutationUpdateUserArgs
 >(async (_, { input: { name, bio } }, { uid, authTrx }) => {
-  const user = await userRepository.findOneBy({
-    id: uid,
-  })
+  const user = await userRepository.findById(uid)
   if (!user) {
     return { errorCodes: [UpdateUserErrorCode.UserNotFound] }
   }
@@ -87,9 +90,7 @@ export const updateUserProfileResolver = authorized<
   UpdateUserProfileError,
   MutationUpdateUserProfileArgs
 >(async (_, { input: { userId, username, pictureUrl } }, { uid, authTrx }) => {
-  const user = await userRepository.findOneBy({
-    id: userId,
-  })
+  const user = await userRepository.findById(userId)
   if (!user) {
     return { errorCodes: [UpdateUserProfileErrorCode.Unauthorized] }
   }
@@ -112,6 +113,7 @@ export const updateUserProfileResolver = authorized<
       profile: {
         username: lowerCasedUsername,
       },
+      status: StatusType.Active,
     })
     if (existingUser?.id) {
       return {
@@ -156,6 +158,7 @@ export const googleLoginResolver: ResolverFn<
 
   const user = await userRepository.findOneBy({
     email,
+    status: StatusType.Active,
   })
   if (!user?.id) {
     return { errorCodes: [LoginErrorCode.UserNotFound] }
@@ -251,9 +254,7 @@ export const getMeUserResolver: ResolverFn<
       return undefined
     }
 
-    const user = await userRepository.findOneBy({
-      id: claims.uid,
-    })
+    const user = await userRepository.findById(claims.uid)
     if (!user) {
       return undefined
     }
@@ -277,12 +278,17 @@ export const getUserResolver: ResolverFn<
   const userId =
     id ||
     (username &&
-      (await userRepository.findOneBy({ profile: { username } }))?.id)
+      (
+        await userRepository.findOneBy({
+          profile: { username },
+          status: StatusType.Active,
+        })
+      )?.id)
   if (!userId) {
     return { errorCodes: [UserErrorCode.UserNotFound] }
   }
 
-  const userRecord = await userRepository.findOneBy({ id: userId })
+  const userRecord = await userRepository.findById(userId)
   if (!userRecord) {
     return { errorCodes: [UserErrorCode.UserNotFound] }
   }
@@ -313,9 +319,10 @@ export const deleteAccountResolver = authorized<
   DeleteAccountSuccess,
   DeleteAccountError,
   MutationDeleteAccountArgs
->(async (_, { userID }, { authTrx, log }) => {
-  const result = await authTrx(async (t) => {
-    return t.withRepository(userRepository).delete(userID)
+>(async (_, { userID }, { log }) => {
+  // soft delete user
+  const result = await updateUser(userID, {
+    status: StatusType.Deleted,
   })
   if (!result.affected) {
     log.error('Error deleting user account')
@@ -334,9 +341,7 @@ export const updateEmailResolver = authorized<
   MutationUpdateEmailArgs
 >(async (_, { input: { email } }, { authTrx, uid, log }) => {
   try {
-    const user = await userRepository.findOneBy({
-      id: uid,
-    })
+    const user = await userRepository.findById(uid)
 
     if (!user) {
       return {

packages/api/src/routers/auth/apple_auth.ts

@@ -15,6 +15,7 @@ import {
   suggestedUsername,
 } from './jwt_helpers'
 import { analytics } from '../../utils/analytics'
+import { StatusType } from '../../entity/user'
 
 const appleBaseURL = 'https://appleid.apple.com'
 const audienceName = 'app.omnivore.app'
@@ -122,6 +123,7 @@ export async function handleAppleWebAuth(
     const user = await userRepository.findOneBy({
       sourceUserId: decodedTokenResult.sourceUserId,
       source: 'APPLE',
+      status: StatusType.Active,
     })
     const userId = user?.id
 

packages/api/src/routers/auth/auth_router.ts

@@ -421,7 +421,7 @@ export function authRouter() {
       const { email, password } = req.body
       try {
         const user = await userRepository.findByEmail(email.trim())
-        if (!user?.id) {
+        if (!user || user.status === StatusType.Deleted) {
           return res.redirect(
             `${env.client.url}/auth/email-login?errorCodes=${LoginErrorCode.UserNotFound}`
           )
@@ -610,7 +610,7 @@ export function authRouter() {
 
       try {
         const user = await userRepository.findByEmail(email)
-        if (!user) {
+        if (!user || user.status === StatusType.Deleted) {
           return res.redirect(`${env.client.url}/auth/reset-sent`)
         }
 

packages/api/src/routers/auth/google_auth.ts

@@ -1,6 +1,7 @@
 import { google, oauth2_v2 as oauthV2 } from 'googleapis'
 import { OAuth2Client } from 'googleapis-common'
 import url from 'url'
+import { StatusType } from '../../entity/user'
 import { env, homePageURL } from '../../env'
 import { LoginErrorCode } from '../../generated/graphql'
 import { userRepository } from '../../repository/user'
@@ -130,6 +131,7 @@ export async function handleGoogleWebAuth(
     const user = await userRepository.findOneBy({
       email,
       source: 'GOOGLE',
+      status: StatusType.Active,
     })
     const userId = user?.id
 

packages/api/src/routers/auth/mobile/sign_in.ts

@@ -46,7 +46,7 @@ export async function createMobileEmailSignInResponse(
     }
 
     const user = await userRepository.findByEmail(email.trim())
-    if (!user?.id || !user?.password) {
+    if (!user || !user.password || user.status === StatusType.Deleted) {
       throw new Error('user not found')
     }
 

packages/api/src/routers/user_router.ts

@@ -39,7 +39,7 @@ export function userRouter() {
       return
     }
     try {
-      const user = await userRepository.findOneBy({ id: claims.uid })
+      const user = await userRepository.findById(claims.uid)
       if (!user) {
         res.status(400).send('Bad Request')
         return

packages/api/src/services/library_item.ts

@@ -438,6 +438,75 @@ export const updateLibraryItem = async (
   return updatedLibraryItem
 }
 
+export const updateLibraryItemReadingProgress = async (
+  id: string,
+  userId: string,
+  bottomPercent: number,
+  topPercent: number | null = null,
+  anchorIndex: number | null = null,
+  pubsub = createPubSubClient()
+): Promise<LibraryItem | null> => {
+  // If we have a top percent, we only save it if it's greater than the current top percent
+  // or set to zero if the top percent is zero.
+  const result = (await authTrx(
+    async (tx) =>
+      tx.getRepository(LibraryItem).query(
+        `
+      UPDATE omnivore.library_item
+      SET reading_progress_top_percent = CASE
+        WHEN reading_progress_top_percent < $2 THEN $2
+        WHEN $2 = 0 THEN 0
+        ELSE reading_progress_top_percent
+      END,
+      reading_progress_bottom_percent = CASE
+        WHEN reading_progress_bottom_percent < $3 THEN $3
+        WHEN $3 = 0 THEN 0
+        ELSE reading_progress_bottom_percent
+      END,
+      reading_progress_highest_read_anchor = CASE
+        WHEN reading_progress_top_percent < $4 THEN $4
+        WHEN $4 = 0 THEN 0
+        ELSE reading_progress_highest_read_anchor
+      END,
+      read_at = now()
+      WHERE id = $1 AND (
+        (reading_progress_top_percent < $2 OR $2 = 0) OR
+        (reading_progress_bottom_percent < $3 OR $3 = 0) OR
+        (reading_progress_highest_read_anchor < $4 OR $4 = 0)
+      )
+      RETURNING
+        id,
+        reading_progress_top_percent as "readingProgressTopPercent",
+        reading_progress_bottom_percent as "readingProgressBottomPercent",
+        reading_progress_highest_read_anchor as "readingProgressHighestReadAnchor",
+        read_at as "readAt"
+      `,
+        [id, topPercent, bottomPercent, anchorIndex]
+      ),
+    undefined,
+    userId
+  )) as [LibraryItem[], number]
+  if (result[1] === 0) {
+    return null
+  }
+
+  const updatedItem = result[0][0]
+  await pubsub.entityUpdated<QueryDeepPartialEntity<LibraryItem>>(
+    EntityType.PAGE,
+    {
+      id,
+      readingProgressBottomPercent: updatedItem.readingProgressBottomPercent,
+      readingProgressTopPercent: updatedItem.readingProgressTopPercent,
+      readingProgressHighestReadAnchor:
+        updatedItem.readingProgressHighestReadAnchor,
+      readAt: updatedItem.readAt,
+    },
+    userId
+  )
+
+  return updatedItem
+}
+
 export const createLibraryItems = async (
   libraryItems: DeepPartial<LibraryItem>[],
   userId: string

packages/api/src/services/newsletters.ts

@@ -21,10 +21,7 @@ export const createNewsletterEmail = async (
   userId: string,
   confirmationCode?: string
 ): Promise<NewsletterEmail> => {
-  const user = await userRepository.findOne({
-    where: { id: userId },
-    relations: ['profile'],
-  })
+  const user = await userRepository.findById(userId)
   if (!user) {
     return Promise.reject({
       errorCode: CreateNewsletterEmailErrorCode.Unauthorized,

packages/api/src/services/save_url.ts

@@ -42,9 +42,7 @@ export const saveUrlFromEmail = async (
   clientRequestId: string,
   userId: string
 ): Promise<boolean> => {
-  const user = await userRepository.findOneBy({
-    id: userId,
-  })
+  const user = await userRepository.findById(userId)
   if (!user) {
     return false
   }

packages/api/src/services/user.ts

@@ -1,4 +1,4 @@
-import { User } from '../entity/user'
+import { StatusType, User } from '../entity/user'
 import { authTrx } from '../repository'
 import { userRepository } from '../repository/user'
 
@@ -13,7 +13,7 @@ export const deleteUser = async (userId: string) => {
 }
 
 export const updateUser = async (userId: string, update: Partial<User>) => {
-  await authTrx(
+  return authTrx(
     async (t) => t.getRepository(User).update(userId, update),
     undefined,
     userId
@@ -21,5 +21,5 @@ export const updateUser = async (userId: string, update: Partial<User>) => {
 }
 
 export const findUser = async (id: string): Promise<User | null> => {
-  return userRepository.findOneBy({ id })
+  return userRepository.findOneBy({ id, status: StatusType.Active })
 }

packages/api/src/utils/parser.ts

@@ -16,6 +16,7 @@ import { ILike } from 'typeorm'
 import { promisify } from 'util'
 import { v4 as uuid } from 'uuid'
 import { Highlight } from '../entity/highlight'
+import { StatusType } from '../entity/user'
 import { env } from '../env'
 import { PageType, PreparedDocumentInput } from '../generated/graphql'
 import { userRepository } from '../repository/user'
@@ -465,6 +466,7 @@ export const isProbablyArticle = async (
 ): Promise<boolean> => {
   const user = await userRepository.findOneBy({
     email: ILike(email),
+    status: StatusType.Active,
   })
   return !!user || subject.includes(ARTICLE_PREFIX)
 }