From 0732fb05dd8441e8e43224fbc4455879b43f8ad9 Mon Sep 17 00:00:00 2001
From: Hongbo Wu <hongbo@omnivore.app>
Date: Thu, 24 Nov 2022 18:39:51 +0800
Subject: [PATCH 1/2] Fix some errors in rule engine

---
 packages/rule-handler/src/filter.ts | 28 +++++++++++++++++-------
 packages/rule-handler/src/label.ts  |  2 +-
 packages/rule-handler/src/rule.ts   | 33 ++++++++++++++++++-----------
 3 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/packages/rule-handler/src/filter.ts b/packages/rule-handler/src/filter.ts
index 6f361db51..e55226a1d 100644
--- a/packages/rule-handler/src/filter.ts
+++ b/packages/rule-handler/src/filter.ts
@@ -9,10 +9,17 @@ interface SearchResponse {
 }
 
 interface Edge {
-  node: Node
+  node: Page
 }
 
-interface Node {
+interface Page {
+  id: string
+  labels: Label[]
+  isArchived: boolean
+  readingProgressPercent: number
+}
+
+interface Label {
   id: string
 }
 
@@ -21,7 +28,7 @@ export const search = async (
   apiEndpoint: string,
   auth: string,
   query: string
-): Promise<Node[]> => {
+): Promise<Page[]> => {
   const requestData = JSON.stringify({
     query: `query Search($query: String) {
               search(query: $query) {
@@ -29,6 +36,11 @@ export const search = async (
                   edges {
                     node {
                       id
+                      labels {
+                        id
+                      }
+                      isArchived
+                      readingProgressPercent      
                     }
                   }
                 }
@@ -55,7 +67,7 @@ export const search = async (
     )
 
     const edges = response.data.data.search.edges
-    if (edges.length == 0) {
+    if (edges.length === 0) {
       return []
     }
 
@@ -67,15 +79,15 @@ export const search = async (
   }
 }
 
-export const isMatched = async (
+export const filterPage = async (
   userId: string,
   apiEndpoint: string,
   auth: string,
   filter: string,
   pageId: string
-): Promise<boolean> => {
+): Promise<Page | null> => {
   filter += ` includes:${pageId}`
-  const nodes = await search(userId, apiEndpoint, auth, filter)
+  const pages = await search(userId, apiEndpoint, auth, filter)
 
-  return nodes.length > 0
+  return pages.length > 0 ? pages[0] : null
 }
diff --git a/packages/rule-handler/src/label.ts b/packages/rule-handler/src/label.ts
index 47c562282..59fa21df2 100644
--- a/packages/rule-handler/src/label.ts
+++ b/packages/rule-handler/src/label.ts
@@ -1,6 +1,6 @@
 import axios from 'axios'
 
-export const addLabels = async (
+export const setLabels = async (
   apiEndpoint: string,
   auth: string,
   pageId: string,
diff --git a/packages/rule-handler/src/rule.ts b/packages/rule-handler/src/rule.ts
index d8dd35f48..c5bc3f2f5 100644
--- a/packages/rule-handler/src/rule.ts
+++ b/packages/rule-handler/src/rule.ts
@@ -1,9 +1,9 @@
 import { sendNotification } from './notification'
 import { getAuthToken, PubSubData } from './index'
 import axios, { AxiosResponse } from 'axios'
-import { addLabels } from './label'
+import { setLabels } from './label'
 import { archivePage, markPageAsRead } from './page'
-import { isMatched } from './filter'
+import { filterPage } from './filter'
 
 export enum RuleActionType {
   AddLabel = 'ADD_LABEL',
@@ -79,26 +79,35 @@ export const triggerActions = async (
   const actionPromises: Promise<AxiosResponse<any, any> | undefined>[] = []
 
   for (const rule of rules) {
-    if (
-      !(await isMatched(userId, apiEndpoint, authToken, rule.filter, data.id))
-    ) {
+    const filteredPage = await filterPage(
+      userId,
+      apiEndpoint,
+      authToken,
+      rule.filter,
+      data.id
+    )
+    if (!filteredPage) {
       continue
     }
 
     rule.actions.forEach((action) => {
       switch (action.type) {
-        case RuleActionType.AddLabel:
-          data.id &&
-            actionPromises.push(
-              addLabels(apiEndpoint, authToken, data.id, action.params)
-            )
+        case RuleActionType.AddLabel: {
+          const existingLabelIds = filteredPage.labels.map((label) => label.id)
+          // combine existing labels with new labels in a set to avoid duplicates
+          const labelIds = new Set([...existingLabelIds, ...action.params])
+
+          actionPromises.push(
+            setLabels(apiEndpoint, authToken, data.id, Array.from(labelIds))
+          )
           break
+        }
         case RuleActionType.Archive:
-          data.id &&
+          !filteredPage.isArchived &&
             actionPromises.push(archivePage(apiEndpoint, authToken, data.id))
           break
         case RuleActionType.MarkAsRead:
-          data.id &&
+          filteredPage.readingProgressPercent < 100 &&
             actionPromises.push(markPageAsRead(apiEndpoint, authToken, data.id))
           break
         case RuleActionType.SendNotification:

From 35e99350a59267272117a5e444bc345828444ade Mon Sep 17 00:00:00 2001
From: Hongbo Wu <hongbo@omnivore.app>
Date: Fri, 25 Nov 2022 10:35:31 +0800
Subject: [PATCH 2/2] Only set label if not exists

---
 packages/rule-handler/src/rule.ts | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/packages/rule-handler/src/rule.ts b/packages/rule-handler/src/rule.ts
index c5bc3f2f5..c8e35a4dc 100644
--- a/packages/rule-handler/src/rule.ts
+++ b/packages/rule-handler/src/rule.ts
@@ -94,8 +94,14 @@ export const triggerActions = async (
       switch (action.type) {
         case RuleActionType.AddLabel: {
           const existingLabelIds = filteredPage.labels.map((label) => label.id)
+          const newLabelIds = action.params
+          if (newLabelIds.every((id) => existingLabelIds.includes(id))) {
+            // All labels are already set
+            return
+          }
+
           // combine existing labels with new labels in a set to avoid duplicates
-          const labelIds = new Set([...existingLabelIds, ...action.params])
+          const labelIds = new Set([...existingLabelIds, ...newLabelIds])
 
           actionPromises.push(
             setLabels(apiEndpoint, authToken, data.id, Array.from(labelIds))