Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Enhance cloud synchronization functionality, support syncing delete operations for messages and conversations, and add support for automatic sync settings #5236

Open
wants to merge 62 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 56 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
1d0a40b
chore: low the google safety setting to avoid unexpected blocking
fredliang44 Dec 31, 2023
78c4084
Merge pull request #4148 from ChatGPTNextWeb/main
fred-bf Feb 27, 2024
1cce87a
Merge pull request #4181 from ChatGPTNextWeb/main
fred-bf Mar 1, 2024
cd354cf
Merge pull request #4685 from ChatGPTNextWeb/main
fred-bf May 14, 2024
d957397
Merge remote-tracking branch 'origin/main' into website
lloydzhou Jul 13, 2024
284d33b
Merge remote-tracking branch 'origin/main' into website
lloydzhou Jul 19, 2024
c440637
Merge remote-tracking branch 'origin/main' into website
lloydzhou Jul 26, 2024
22f6129
fix: Fixed an issue where the sample of the reply content was display…
Aug 2, 2024
5065091
fix: Fixed the issue that WebDAV synchronization could not check the …
Aug 3, 2024
22c7959
feat: The cloud synchronization feature is enhanced to support the sy…
Aug 3, 2024
faac0d9
Merge remote-tracking branch 'origin/main' into website
lloydzhou Aug 6, 2024
4f876f3
Merge tag 'v2.14.1' into website
Aug 8, 2024
648e600
feat: The cloud synchronization feature is enhanced to support the sy…
Aug 3, 2024
93bfb55
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Aug 14, 2024
4b22aaf
feat: Add automatic data synchronization settings and implementation,…
Aug 15, 2024
621b148
Merge branch 'ChatGPTNextWeb:main' into main
ahzmr Aug 15, 2024
eae593d
feat: Add automatic data synchronization settings and implementation,…
Aug 15, 2024
2ee2d50
Merge remote-tracking branch 'up/website' into website
Aug 15, 2024
5e1064a
Merge branch 'main' into website
lloydzhou Aug 16, 2024
0a6ddda
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Aug 17, 2024
b2336f5
更新docker.yml, 修改自动编译的镜像为自己的账号
ahzmr Aug 18, 2024
31f2829
Merge remote-tracking branch 'up/website' into website
ahzmr Aug 18, 2024
e515f0f
更新docker.yml, 修改自动编译的镜像为自己的账号
ahzmr Aug 18, 2024
fdb89af
更新docker.yml,使image名自适应,不影响主仓库
ahzmr Aug 18, 2024
fc97c4b
更新docker.yml,使image名自适应,不影响主仓库
ahzmr Aug 18, 2024
0745b64
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Aug 20, 2024
d0b7ddc
feat: 优化会话列表按最后更新时间倒序排序,更方便查看与管理
Aug 20, 2024
5c51fd2
feat: 优化会话列表按最后更新时间倒序排序,更方便查看与管理
Aug 20, 2024
2fdb35b
fix: 解决会话列表按最新操作时间倒序排序,当前会话判断失败的bug
Aug 20, 2024
31baa10
fix: 解决会话列表按最新操作时间倒序排序,当前会话判断失败的bug
Aug 20, 2024
f1d69cb
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Aug 21, 2024
2d68f17
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Aug 22, 2024
0638db1
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Aug 25, 2024
e8c7ac0
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Aug 28, 2024
2bf72d0
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Aug 30, 2024
c204031
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Sep 5, 2024
ccacfec
feat: 优化聊天窗口,使支持复制会话
ahzmr Sep 5, 2024
6dc8681
fix: 优化云同步功能,使access配置按更新时间合并,解决自定义模型配置在同步后丢失的问题
ahzmr Sep 5, 2024
6f3d753
Merge remote-tracking branch 'origin/main' into website
lloydzhou Sep 6, 2024
5ae4921
fix: 优化云同步功能,自动去除掉非首个空会话,避免多个空会话在中间,更方便管理
ahzmr Sep 6, 2024
370ce3e
Merge remote-tracking branch 'up/website' into website
ahzmr Sep 7, 2024
9551f5d
Merge branch 'website'
ahzmr Sep 7, 2024
35f5288
Merge remote-tracking branch 'up/main'
ahzmr Sep 12, 2024
144fdc9
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Sep 13, 2024
659a389
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Sep 14, 2024
60bd3c5
Merge remote-tracking branch 'up/main'
ahzmr Sep 26, 2024
89edebd
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Sep 28, 2024
41242ca
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Sep 30, 2024
cf7c6f2
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Oct 1, 2024
c6657d3
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Oct 4, 2024
31900cb
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Oct 7, 2024
c4ae73d
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Oct 9, 2024
9a025ae
Merge tag 'v2.15.4'
ahzmr Oct 9, 2024
98ab561
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Oct 10, 2024
f80da8a
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Oct 11, 2024
3e02a71
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Oct 27, 2024
f45a693
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Oct 29, 2024
7f3ec6d
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Oct 30, 2024
dfd3d24
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Oct 31, 2024
af23929
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Nov 1, 2024
9a95d32
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Nov 2, 2024
b2381b2
Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next…
actions-user Nov 5, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions .github/workflows/docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,26 @@ jobs:
with:
username: ${{ secrets.DOCKER_USERNAME }}
password: ${{ secrets.DOCKER_PASSWORD }}
-

-
name: Extract metadata (tags, labels) for Docker
id: meta
uses: docker/metadata-action@v4
with:
images: yidadaa/chatgpt-next-web
images: ${{ secrets.DOCKER_USERNAME }}/chatgpt-next-web
tags: |
type=raw,value=latest
type=ref,event=tag
-

-
name: Set up QEMU
uses: docker/setup-qemu-action@v2

-
-
name: Set up Docker Buildx
uses: docker/setup-buildx-action@v2
-

-
name: Build and push Docker image
uses: docker/build-push-action@v4
with:
Expand All @@ -49,4 +49,4 @@ jobs:
labels: ${{ steps.meta.outputs.labels }}
cache-from: type=gha
cache-to: type=gha,mode=max

1 change: 1 addition & 0 deletions app/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export function useCommand(commands: Commands = {}) {
interface ChatCommands {
new?: Command;
newm?: Command;
copy?: Command;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Incomplete copy Command Implementation in exporter.tsx

  • The copy command is defined as a no-operation function in app/components/exporter.tsx:
    copy: () => void;
    
    This may result in the copy functionality not performing as expected within the exporter component.
🔗 Analysis chain

LGTM! New copy command added successfully.

The addition of the copy command to the ChatCommands interface is implemented correctly and consistently with other commands. This aligns with the PR objective of enhancing functionality.

To ensure proper implementation of the new copy command, please run the following script:

This script will help verify that the copy command is properly implemented and used in other parts of the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation and usage of the new 'copy' command

# Test 1: Check for the implementation of the 'copy' command
echo "Checking for 'copy' command implementation:"
rg --type typescript "copy\s*:\s*\([^)]*\)\s*=>" -g '!app/command.ts'

# Test 2: Check for usage of the 'copy' command
echo "Checking for 'copy' command usage:"
rg --type typescript "copy\s*:" -g '!app/command.ts'

# Test 3: Check for any related UI elements or text
echo "Checking for related UI elements or text:"
rg --type typescript "copy" -g '!app/command.ts'

Length of output: 561


Script:

#!/bin/bash
# Description: Verify the implementation and usage of the new 'copy' command

# Test 1: Check for the implementation of the 'copy' command
echo "Checking for 'copy' command implementation:"
rg "copy\s*:\s*\([^)]*\)\s*=>" -g '*.ts' -g '*.tsx'

# Test 2: Check for usage of the 'copy' command
echo "Checking for 'copy' command usage:"
rg "copy\s*:" -g '*.ts' -g '*.tsx'

# Test 3: Check for any related UI elements or text
echo "Checking for related UI elements or text:"
rg "copy" -g '*.ts' -g '*.tsx'

Length of output: 7136

next?: Command;
prev?: Command;
clear?: Command;
Expand Down
20 changes: 16 additions & 4 deletions app/components/chat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ import {
getMessageImages,
isVisionModel,
isDalle3,
removeOutdatedEntries,
showPlugins,
safeLocalStorage,
} from "../utils";
Expand Down Expand Up @@ -987,6 +988,7 @@ function _Chat() {
const chatCommands = useChatCommand({
new: () => chatStore.newSession(),
newm: () => navigate(Path.NewChat),
copy: () => chatStore.copySession(),
prev: () => chatStore.nextSession(-1),
next: () => chatStore.nextSession(1),
clear: () =>
Expand Down Expand Up @@ -1118,10 +1120,20 @@ function _Chat() {
};

const deleteMessage = (msgId?: string) => {
chatStore.updateCurrentSession(
(session) =>
(session.messages = session.messages.filter((m) => m.id !== msgId)),
);
chatStore.updateCurrentSession((session) => {
session.deletedMessageIds &&
removeOutdatedEntries(session.deletedMessageIds);
session.messages = session.messages.filter((m) => {
Comment on lines +1125 to +1126
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Initialize deletedMessageIds before usage

In the deleteMessage function, session.deletedMessageIds is used before it's ensured to be initialized. If session.deletedMessageIds is undefined, calling removeOutdatedEntries(session.deletedMessageIds); could lead to errors. It's important to initialize session.deletedMessageIds before using it to prevent potential runtime exceptions.

Please apply the following diff to initialize deletedMessageIds before use:

const deleteMessage = (msgId?: string) => {
  chatStore.updateCurrentSession((session) => {
+   if (!session.deletedMessageIds) {
+     session.deletedMessageIds = {} as Record<string, number>;
+   }
    removeOutdatedEntries(session.deletedMessageIds);
    session.messages = session.messages.filter((m) => {
      if (m.id !== msgId) {
        return true;
      }
      session.deletedMessageIds[m.id] = Date.now();
      return false;
    });
  });
};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
removeOutdatedEntries(session.deletedMessageIds);
session.messages = session.messages.filter((m) => {
const deleteMessage = (msgId?: string) => {
chatStore.updateCurrentSession((session) => {
if (!session.deletedMessageIds) {
session.deletedMessageIds = {} as Record<string, number>;
}
removeOutdatedEntries(session.deletedMessageIds);
session.messages = session.messages.filter((m) => {

if (m.id !== msgId) {
return true;
}
if (!session.deletedMessageIds) {
session.deletedMessageIds = {} as Record<string, number>;
}
session.deletedMessageIds[m.id] = Date.now();
return false;
});
});
Comment on lines +1123 to +1136
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Refactor to avoid side effects within the filter method

Modifying session.deletedMessageIds inside the filter callback introduces side effects, which can lead to unexpected behavior and reduces code readability. It's generally best practice to avoid side effects within array iteration methods like filter.

Consider refactoring the code to separate the side effects from the filtering logic:

chatStore.updateCurrentSession((session) => {
+  if (!session.deletedMessageIds) {
+    session.deletedMessageIds = {} as Record<string, number>;
+  }
+  session.deletedMessageIds && removeOutdatedEntries(session.deletedMessageIds);

-  session.messages = session.messages.filter((m) => {
-    if (m.id !== msgId) {
-      return true;
-    }
-    session.deletedMessageIds[m.id] = Date.now();
-    return false;
-  });
+  session.messages = session.messages.filter((m) => m.id !== msgId);
+
+  if (msgId) {
+    session.deletedMessageIds[msgId] = Date.now();
+  }
});

This refactoring improves readability by:

  • Initializing session.deletedMessageIds before the filter.
  • Separating the deletion logic from the message filtering.
  • Avoiding side effects within the filter method.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
chatStore.updateCurrentSession((session) => {
session.deletedMessageIds &&
removeOutdatedEntries(session.deletedMessageIds);
session.messages = session.messages.filter((m) => {
if (m.id !== msgId) {
return true;
}
if (!session.deletedMessageIds) {
session.deletedMessageIds = {} as Record<string, number>;
}
session.deletedMessageIds[m.id] = Date.now();
return false;
});
});
chatStore.updateCurrentSession((session) => {
if (!session.deletedMessageIds) {
session.deletedMessageIds = {} as Record<string, number>;
}
session.deletedMessageIds && removeOutdatedEntries(session.deletedMessageIds);
session.messages = session.messages.filter((m) => m.id !== msgId);
if (msgId) {
session.deletedMessageIds[msgId] = Date.now();
}
});

};

const onDelete = (msgId: string) => {
Expand Down
15 changes: 15 additions & 0 deletions app/components/settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,21 @@ function SyncConfigModal(props: { onClose?: () => void }) {
</select>
</ListItem>

<ListItem
title={Locale.Settings.Sync.Config.EnableAutoSync.Title}
subTitle={Locale.Settings.Sync.Config.EnableAutoSync.SubTitle}
>
<input
type="checkbox"
checked={syncStore.enableAutoSync}
onChange={(e) => {
syncStore.update(
(config) => (config.enableAutoSync = e.currentTarget.checked),
);
Comment on lines +373 to +375
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid assignments within expressions.

The assignment within the expression can lead to confusion and potential side effects. Consider refactoring to separate the assignment from the update function call.

- syncStore.update(
-   (config) => (config.enableAutoSync = e.currentTarget.checked),
- );
+ const enableAutoSync = e.currentTarget.checked;
+ syncStore.update((config) => {
+   config.enableAutoSync = enableAutoSync;
+ });
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
syncStore.update(
(config) => (config.enableAutoSync = e.currentTarget.checked),
);
const enableAutoSync = e.currentTarget.checked;
syncStore.update((config) => {
config.enableAutoSync = enableAutoSync;
});
Tools
Biome

[error] 369-369: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

}}
></input>
</ListItem>
Comment on lines +365 to +378
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Refactor the assignment within the expression.

The implementation of the automatic synchronization toggle looks good overall. However, there's a minor issue with the assignment within the expression in the onChange handler.

To improve readability and avoid potential side effects, consider refactoring the onChange handler as follows:

 onChange={(e) => {
-  syncStore.update(
-    (config) => (config.enableAutoSync = e.currentTarget.checked),
-  );
+  const enableAutoSync = e.currentTarget.checked;
+  syncStore.update((config) => {
+    config.enableAutoSync = enableAutoSync;
+  });
 }}

This change separates the assignment from the update function call, making the code clearer and less prone to unexpected behavior.

The overall implementation of the automatic synchronization feature is well-integrated into the existing UI and follows the established patterns in the codebase.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<ListItem
title={Locale.Settings.Sync.Config.EnableAutoSync.Title}
subTitle={Locale.Settings.Sync.Config.EnableAutoSync.SubTitle}
>
<input
type="checkbox"
checked={syncStore.enableAutoSync}
onChange={(e) => {
syncStore.update(
(config) => (config.enableAutoSync = e.currentTarget.checked),
);
}}
></input>
</ListItem>
<ListItem
title={Locale.Settings.Sync.Config.EnableAutoSync.Title}
subTitle={Locale.Settings.Sync.Config.EnableAutoSync.SubTitle}
>
<input
type="checkbox"
checked={syncStore.enableAutoSync}
onChange={(e) => {
const enableAutoSync = e.currentTarget.checked;
syncStore.update((config) => {
config.enableAutoSync = enableAutoSync;
});
}}
></input>
</ListItem>
🧰 Tools
🪛 Biome

[error] 372-372: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


<ListItem
title={Locale.Settings.Sync.Config.Proxy.Title}
subTitle={Locale.Settings.Sync.Config.Proxy.SubTitle}
Expand Down
5 changes: 5 additions & 0 deletions app/locales/cn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ const cn = {
Commands: {
new: "新建聊天",
newm: "从面具新建聊天",
copy: "复制当前聊天",
next: "下一个聊天",
prev: "上一个聊天",
clear: "清除上下文",
Expand Down Expand Up @@ -234,6 +235,10 @@ const cn = {
Title: "同步类型",
SubTitle: "选择喜爱的同步服务器",
},
EnableAutoSync: {
Title: "自动同步设置",
SubTitle: "在回复完成或删除消息后自动同步数据",
},
Proxy: {
Title: "启用代理",
SubTitle: "在浏览器中同步时,必须启用代理以避免跨域限制",
Expand Down
6 changes: 6 additions & 0 deletions app/locales/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const en: LocaleType = {
Commands: {
new: "Start a new chat",
newm: "Start a new chat with mask",
copy: "Copy the current Chat",
next: "Next Chat",
prev: "Previous Chat",
clear: "Clear Context",
Expand Down Expand Up @@ -236,6 +237,11 @@ const en: LocaleType = {
Title: "Sync Type",
SubTitle: "Choose your favorite sync service",
},
EnableAutoSync: {
Title: "Auto Sync Settings",
SubTitle:
"Automatically synchronize data after replying or deleting messages",
},
Proxy: {
Title: "Enable CORS Proxy",
SubTitle: "Enable a proxy to avoid cross-origin restrictions",
Expand Down
2 changes: 1 addition & 1 deletion app/store/access.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ export const useAccessStore = createPersistStore(
})
.then((res: DangerConfig) => {
console.log("[Config] got config from server", res);
set(() => ({ ...res }));
set(() => ({ lastUpdateTime: Date.now(), ...res }));
})
.catch(() => {
console.error("[Config] failed to fetch config");
Expand Down
79 changes: 77 additions & 2 deletions app/store/chat.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { getMessageTextContent, trimTopic } from "../utils";
import {
getMessageTextContent,
trimTopic,
removeOutdatedEntries,
} from "../utils";

import { indexedDBStorage } from "@/app/utils/indexedDB-storage";
import { nanoid } from "nanoid";
Expand Down Expand Up @@ -29,6 +33,7 @@ import { ModelConfig, ModelType, useAppConfig } from "./config";
import { useAccessStore } from "./access";
import { collectModelsWithDefaultModel } from "../utils/model";
import { createEmptyMask, Mask } from "./mask";
import { useSyncStore } from "./sync";

const localStorage = safeLocalStorage();

Expand Down Expand Up @@ -80,6 +85,7 @@ export interface ChatSession {
lastUpdate: number;
lastSummarizeIndex: number;
clearContextIndex?: number;
deletedMessageIds?: Record<string, number>;

mask: Mask;
}
Expand All @@ -103,6 +109,7 @@ function createEmptySession(): ChatSession {
},
lastUpdate: Date.now(),
lastSummarizeIndex: 0,
deletedMessageIds: {},

mask: createEmptyMask(),
};
Expand Down Expand Up @@ -188,9 +195,19 @@ function fillTemplateWith(input: string, modelConfig: ModelConfig) {
return output;
}

let cloudSyncTimer: any = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Specify a more precise type for cloudSyncTimer instead of any

Using the any type reduces type safety and can lead to potential runtime errors. Consider specifying a more accurate type for cloudSyncTimer to enhance type checking and maintainability.

Apply this diff to specify a more precise type:

-let cloudSyncTimer: any = null;
+let cloudSyncTimer: ReturnType<typeof setTimeout> | null = null;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let cloudSyncTimer: any = null;
let cloudSyncTimer: ReturnType<typeof setTimeout> | null = null;

function noticeCloudSync(): void {
const syncStore = useSyncStore.getState();
cloudSyncTimer && clearTimeout(cloudSyncTimer);
cloudSyncTimer = setTimeout(() => {
syncStore.autoSync();
}, 500);
}

const DEFAULT_CHAT_STATE = {
sessions: [createEmptySession()],
currentSessionIndex: 0,
deletedSessionIds: {} as Record<string, number>,
lastInput: "",
};

Expand Down Expand Up @@ -240,6 +257,28 @@ export const useChatStore = createPersistStore(
});
},

copySession() {
set((state) => {
const { sessions, currentSessionIndex } = state;
const emptySession = createEmptySession();

// copy the session
const curSession = JSON.parse(
JSON.stringify(sessions[currentSessionIndex]),
);
curSession.id = emptySession.id;
curSession.lastUpdate = emptySession.lastUpdate;

const newSessions = [...sessions];
newSessions.splice(0, 0, curSession);

return {
currentSessionIndex: 0,
sessions: newSessions,
};
});
},
Comment on lines +260 to +280
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid using JSON.parse(JSON.stringify(...)) for deep cloning

Using JSON.parse(JSON.stringify(...)) for deep cloning can lead to issues, especially with functions, undefined values, or symbols in the object. It can also have performance drawbacks for large objects. Consider using structuredClone or a utility library for deep cloning to ensure all properties are copied correctly.

Apply this diff to use structuredClone for deep cloning:

-const curSession = JSON.parse(
-  JSON.stringify(sessions[currentSessionIndex]),
-);
+const curSession = structuredClone(sessions[currentSessionIndex]);

If structuredClone is not available in your environment, you might use a utility like lodash.cloneDeep.

+import cloneDeep from 'lodash.clonedeep';
+
 const curSession = cloneDeep(sessions[currentSessionIndex]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
copySession() {
set((state) => {
const { sessions, currentSessionIndex } = state;
const emptySession = createEmptySession();
// copy the session
const curSession = JSON.parse(
JSON.stringify(sessions[currentSessionIndex]),
);
curSession.id = emptySession.id;
curSession.lastUpdate = emptySession.lastUpdate;
const newSessions = [...sessions];
newSessions.splice(0, 0, curSession);
return {
currentSessionIndex: 0,
sessions: newSessions,
};
});
},
copySession() {
set((state) => {
const { sessions, currentSessionIndex } = state;
const emptySession = createEmptySession();
// copy the session
const curSession = structuredClone(sessions[currentSessionIndex]);
curSession.id = emptySession.id;
curSession.lastUpdate = emptySession.lastUpdate;
const newSessions = [...sessions];
newSessions.splice(0, 0, curSession);
return {
currentSessionIndex: 0,
sessions: newSessions,
};
});
},


moveSession(from: number, to: number) {
set((state) => {
const { sessions, currentSessionIndex: oldIndex } = state;
Expand Down Expand Up @@ -302,7 +341,18 @@ export const useChatStore = createPersistStore(
if (!deletedSession) return;

const sessions = get().sessions.slice();
sessions.splice(index, 1);
const deletedSessionIds = { ...get().deletedSessionIds };

removeOutdatedEntries(deletedSessionIds);

const hasDelSessions = sessions.splice(index, 1);
if (hasDelSessions?.length) {
hasDelSessions.forEach((session) => {
if (session.messages.length > 0) {
deletedSessionIds[session.id] = Date.now();
}
});
}

const currentIndex = get().currentSessionIndex;
let nextIndex = Math.min(
Expand All @@ -319,19 +369,24 @@ export const useChatStore = createPersistStore(
const restoreState = {
currentSessionIndex: get().currentSessionIndex,
sessions: get().sessions.slice(),
deletedSessionIds: get().deletedSessionIds,
};

set(() => ({
currentSessionIndex: nextIndex,
sessions,
deletedSessionIds,
}));

noticeCloudSync();

showToast(
Locale.Home.DeleteToast,
{
text: Locale.Home.Revert,
onClick() {
set(() => restoreState);
noticeCloudSync();
},
},
5000,
Expand All @@ -352,13 +407,33 @@ export const useChatStore = createPersistStore(
return session;
},

sortSessions() {
const currentSession = get().currentSession();
const sessions = get().sessions.slice();

sessions.sort(
(a, b) =>
new Date(b.lastUpdate).getTime() - new Date(a.lastUpdate).getTime(),
);
Comment on lines +414 to +417
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Simplify session sorting by using numeric comparison

Since lastUpdate is already a timestamp, you can simplify the sorting logic by directly comparing the numbers without creating Date objects, which improves performance.

Apply this diff to refine the sorting function:

sessions.sort(
-  (a, b) =>
-    new Date(b.lastUpdate).getTime() - new Date(a.lastUpdate).getTime(),
+  (a, b) => b.lastUpdate - a.lastUpdate,
);

Committable suggestion was skipped due to low confidence.

const currentSessionIndex = sessions.findIndex((session) => {
return session && currentSession && session.id === currentSession.id;
});

set((state) => ({
currentSessionIndex,
sessions,
}));
},
Comment on lines +410 to +426
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix session comparison and optimize sorting implementation.

There are two issues in the sorting implementation:

  1. The session ID comparison appears to be comparing against itself
  2. Creating Date objects for comparison is unnecessary since lastUpdate is already a timestamp

Apply these fixes:

 sessions.sort(
-  (a, b) =>
-    new Date(b.lastUpdate).getTime() - new Date(a.lastUpdate).getTime(),
+  (a, b) => b.lastUpdate - a.lastUpdate,
 );
 const currentSessionIndex = sessions.findIndex((session) => {
-  return session && currentSession && session.id === currentSession.id;
+  return session && currentSession && session.id === currentSession?.id;
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
sortSessions() {
const currentSession = get().currentSession();
const sessions = get().sessions.slice();
sessions.sort(
(a, b) =>
new Date(b.lastUpdate).getTime() - new Date(a.lastUpdate).getTime(),
);
const currentSessionIndex = sessions.findIndex((session) => {
return session && currentSession && session.id === currentSession.id;
});
set((state) => ({
currentSessionIndex,
sessions,
}));
},
sortSessions() {
const currentSession = get().currentSession();
const sessions = get().sessions.slice();
sessions.sort(
(a, b) => b.lastUpdate - a.lastUpdate,
);
const currentSessionIndex = sessions.findIndex((session) => {
return session && currentSession && session.id === currentSession?.id;
});
set((state) => ({
currentSessionIndex,
sessions,
}));
},


onNewMessage(message: ChatMessage) {
get().updateCurrentSession((session) => {
session.messages = session.messages.concat();
session.lastUpdate = Date.now();
});
get().updateStat(message);
get().summarizeSession();
get().sortSessions();
noticeCloudSync();
},

async onUserInput(content: string, attachImages?: string[]) {
Expand Down
25 changes: 22 additions & 3 deletions app/store/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export type SyncStore = GetStoreState<typeof useSyncStore>;

const DEFAULT_SYNC_STATE = {
provider: ProviderType.WebDAV,
enableAutoSync: true,
useProxy: true,
proxyUrl: ApiPath.Cors as string,

Expand All @@ -43,6 +44,8 @@ const DEFAULT_SYNC_STATE = {
lastProvider: "",
};

let lastSyncTime = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Inconsistent handling of 'lastSyncTime'

The declaration of let lastSyncTime = 0; at the module level introduces a separate variable that may conflict with the lastSyncTime stored in the state (DEFAULT_SYNC_STATE). This inconsistency can lead to unexpected behavior, as different parts of the code may reference different values of lastSyncTime.

Suggestion: Use the state variable 'lastSyncTime' consistently

Consider removing the module-level declaration of lastSyncTime and using the state variable throughout the code to maintain consistency.

Apply this diff to address the issue:

- let lastSyncTime = 0;

... Inside the 'sync' method:
- if (lastSyncTime && lastSyncTime >= Date.now() - 800) {
+ if (get().lastSyncTime && get().lastSyncTime >= Date.now() - 800) {

...

- lastSyncTime = Date.now();
+ set({ lastSyncTime: Date.now() });

Committable suggestion was skipped due to low confidence.


export const useSyncStore = createPersistStore(
DEFAULT_SYNC_STATE,
(set, get) => ({
Expand Down Expand Up @@ -89,6 +92,16 @@ export const useSyncStore = createPersistStore(
},

async sync() {
if (lastSyncTime && lastSyncTime >= Date.now() - 800) {
return;
}
lastSyncTime = Date.now();

const enableAutoSync = get().enableAutoSync;
if (!enableAutoSync) {
return;
}

Comment on lines +95 to +104
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure consistent use of state for 'lastSyncTime' and 'enableAutoSync'

Within the sync method, lastSyncTime is used from the module-level variable, while enableAutoSync is retrieved from the state using get(). This inconsistency can cause synchronization issues and makes the code harder to maintain.

Suggestion: Retrieve both 'lastSyncTime' and 'enableAutoSync' from the state

Consistently using the state ensures that all parts of the code are referencing the same source of truth.

Apply this diff to make the necessary changes:

- if (lastSyncTime && lastSyncTime >= Date.now() - 800) {
+ const { lastSyncTime, enableAutoSync } = get();
+ if (lastSyncTime && lastSyncTime >= Date.now() - 800) {

...

- lastSyncTime = Date.now();
+ set({ lastSyncTime: Date.now() });

...

- const enableAutoSync = get().enableAutoSync;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (lastSyncTime && lastSyncTime >= Date.now() - 800) {
return;
}
lastSyncTime = Date.now();
const enableAutoSync = get().enableAutoSync;
if (!enableAutoSync) {
return;
}
const { lastSyncTime, enableAutoSync } = get();
if (lastSyncTime && lastSyncTime >= Date.now() - 800) {
return;
}
set({ lastSyncTime: Date.now() });
if (!enableAutoSync) {
return;
}

const localState = getLocalAppState();
const provider = get().provider;
const config = get()[provider];
Expand All @@ -103,9 +116,7 @@ export const useSyncStore = createPersistStore(
);
return;
} else {
const parsedRemoteState = JSON.parse(
await client.get(config.username),
) as AppState;
const parsedRemoteState = JSON.parse(remoteState) as AppState;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling when parsing 'remoteState'

Parsing remoteState without proper error handling can lead to uncaught exceptions if the data is not valid JSON. This could interrupt the synchronization process and affect the user experience.

Suggestion: Implement try-catch block for JSON parsing

Wrap the parsing logic in a try-catch block to handle potential errors gracefully.

Apply this diff:

- const parsedRemoteState = JSON.parse(remoteState) as AppState;
- mergeAppState(localState, parsedRemoteState);
+ try {
+   const parsedRemoteState = JSON.parse(remoteState) as AppState;
+   mergeAppState(localState, parsedRemoteState);
+ } catch (e) {
+   console.error("[Sync] Failed to parse remote state", e);
+   showToast(Locale.Settings.Sync.ParseRemoteFailed);
+   // Optionally handle the error, e.g., by using the local state or prompting the user
+   return;
+ }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const parsedRemoteState = JSON.parse(remoteState) as AppState;
try {
const parsedRemoteState = JSON.parse(remoteState) as AppState;
mergeAppState(localState, parsedRemoteState);
} catch (e) {
console.error("[Sync] Failed to parse remote state", e);
showToast(Locale.Settings.Sync.ParseRemoteFailed);
// Optionally handle the error, e.g., by using the local state or prompting the user
return;
}

mergeAppState(localState, parsedRemoteState);
setLocalAppState(localState);
}
Expand All @@ -123,6 +134,14 @@ export const useSyncStore = createPersistStore(
const client = this.getClient();
return await client.check();
},

async autoSync() {
const { lastSyncTime, provider } = get();
const syncStore = useSyncStore.getState();
if (lastSyncTime && syncStore.cloudSync()) {
syncStore.sync();
}
},
Comment on lines +138 to +144
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Simplify 'autoSync' method by using 'this' context and 'get()'

Currently, the autoSync method accesses the store state using get() and useSyncStore.getState(), and calls syncStore.sync(). Since autoSync is within the store context, you can simplify the code by using this and get() directly.

Consider refactoring autoSync as follows:

async autoSync() {
-  const { lastSyncTime, provider } = get();
-  const syncStore = useSyncStore.getState();
-  if (lastSyncTime && syncStore.cloudSync()) {
-    syncStore.sync();
+  const { lastSyncTime } = get();
+  if (lastSyncTime && this.cloudSync()) {
+    await this.sync();
   }
}

This refactor makes the code more consistent and leverages the this context within the store.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async autoSync() {
const { lastSyncTime, provider } = get();
const syncStore = useSyncStore.getState();
if (lastSyncTime && syncStore.cloudSync()) {
syncStore.sync();
}
},
async autoSync() {
const { lastSyncTime } = get();
if (lastSyncTime && this.cloudSync()) {
await this.sync();
}
},

}),
{
name: StoreKey.Sync,
Expand Down
Loading