feat: external sticker cache #29

ruattd · 2024-01-24T10:45:12Z

实验性功能: 在外部存储缓存贴纸以方便与其他应用共享 (如 QQ 等)

~~我不确定它会不会炸因为我没法编译 Nagram~~

sourcery-ai

PR Type: Enhancement

PR Summary: The pull request introduces an experimental feature that allows external storage caching of stickers for sharing with other applications, such as QQ. It includes changes to the MediaDataController to handle the caching logic and adds a new ConfigCellButton class to manage button cells in the settings activity. Additionally, it modifies the NekoExperimentalSettingsActivity to integrate the new button and its associated actions.

Decision: Comment

📝 Type: 'Enhancement' - not supported yet.

Sourcery currently only approves 'Typo fix' PRs.

✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.

No details provided.

📝 Complexity: the changes are too large or complex for Sourcery to approve.

Unsupported files: the diff contains files that Sourcery does not currently support during reviews.

General suggestions:

Review the lifecycle management of the new UI components to ensure they are properly cleaned up to prevent memory leaks.
Consider the appropriate timing and lifecycle events for refreshing the external sticker storage state to ensure it is updated accurately.
Ensure that the new feature's error handling is robust, especially in the caching process, to prevent application crashes.
Check for null references in callback methods to avoid null pointer exceptions.
The PR description could be improved by providing more context on the feature's impact and confirming that it has been tested, as the current description expresses uncertainty about the feature's stability.

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

TMessagesProj/src/main/java/tw/nekomimi/nekogram/settings/NekoExperimentalSettingsActivity.java

sourcery-ai · 2024-01-24T10:51:12Z

TMessagesProj/src/main/java/tw/nekomimi/nekogram/settings/NekoExperimentalSettingsActivity.java

@@ -133,6 +152,8 @@ public void onItemClick(int id) {
            }
        });

+        refreshExternalStickerStorageState(); // Cell (externalStickerCacheRow): Refresh state


suggestion (llm): Consider moving the call to refreshExternalStickerStorageState to a more appropriate lifecycle method if onFragmentCreate is not the best place to refresh the state, ensuring that the state is refreshed at the correct time.

TMessagesProj/src/main/java/tw/nekomimi/nekogram/settings/NekoExperimentalSettingsActivity.java

TMessagesProj/src/main/java/org/telegram/messenger/MediaDataController.java

TMessagesProj/src/main/java/tw/nekomimi/nekogram/config/cell/ConfigCellButton.java

NextAlone · 2024-01-24T16:25:32Z

@ruattd 加一个保存到相册不就行了

NextAlone · 2024-01-24T16:25:53Z

ruattd · 2024-01-24T16:35:32Z

只是我太懒了想让它自动完成这个过程（外加隔壁 qa 有人写的阴间代码不从相册里面读

sourcery-ai

PR Type: Enhancement

PR Summary: The pull request introduces a new experimental feature that allows caching stickers in external storage for sharing with other applications. It includes changes to various classes to support this functionality, such as adding new menu options for managing the external sticker cache and handling the selection of external directories for the cache.

Decision: Comment

📝 Type: 'Enhancement' - not supported yet.

Sourcery currently only approves 'Typo fix' PRs.

✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.

No details provided.

📝 Complexity: the changes are too large or complex for Sourcery to approve.

Unsupported files: the diff contains files that Sourcery does not currently support during reviews.

General suggestions:

Review the synchronous calls to cacheStickers to ensure they do not adversely affect the main thread's performance. Consider using asynchronous patterns if necessary.
Add error handling for cases where the ACTION_OPEN_DOCUMENT_TREE intent cannot be resolved by any app on the device to prevent application crashes.
Clarify the usage of String() method on getExternalStickerCache() to ensure it is the correct way to check if the external sticker cache configuration is not blank.
Ensure that the new feature is well-documented within the codebase to help future maintainers understand its purpose and implementation.
Verify that all new user-facing features, such as menu options for managing the external sticker cache, are intuitive and provide a good user experience.

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

sourcery-ai · 2024-01-30T07:59:28Z

TMessagesProj/src/main/java/tw/nekomimi/nekogram/settings/NekoExperimentalSettingsActivity.java

+            NaConfig.INSTANCE.getExternalStickerCache().setConfigString("");
+        } else {
+            Intent intent = new Intent(Intent.ACTION_OPEN_DOCUMENT_TREE);
+            startActivityForResult(intent, INTENT_PICK_EXTERNAL_STICKER_DIRECTORY);


suggestion (llm): It's good practice to handle the potential ActivityNotFoundException that might be thrown if no app can handle the ACTION_OPEN_DOCUMENT_TREE intent.

sourcery-ai · 2024-01-30T07:59:28Z

TMessagesProj/src/main/java/org/telegram/messenger/MediaDataController.java

@@ -1952,6 +1953,9 @@ public void addNewStickerSet(TLRPC.TL_messages_stickerSet set) {
        loadHash[type] = calcStickersHash(stickerSets[type]);
        getNotificationCenter().postNotificationName(NotificationCenter.stickersDidLoad, type, true);
        loadStickers(type, false, true);
+
+        // Na: [ExternalStickerCache] cache sticker sets
+        ExternalStickerCacheHelper.cacheStickers();


suggestion (llm): Consider the impact of invoking cacheStickers method synchronously here. If this operation is I/O intensive or time-consuming, it might be beneficial to run it in a separate thread or use an asynchronous approach to avoid blocking the main thread.

sourcery-ai · 2024-01-30T07:59:28Z

TMessagesProj/src/main/java/org/telegram/ui/Components/StickersAlert.java

@@ -865,6 +869,10 @@ public void requestLayout() {
        optionsButton.addSubItem(2, R.drawable.msg_link, LocaleController.getString("CopyLink", R.string.CopyLink));
        optionsButton.addSubItem(3, R.drawable.msg_qrcode, LocaleController.getString("ShareQRCode", R.string.ShareQRCode));
        optionsButton.addSubItem(menu_archive, R.drawable.msg_archive, LocaleController.getString("Archive", R.string.Archive));
+        if (!NaConfig.INSTANCE.getExternalStickerCache().String().isBlank()) {


question (llm): The method call String() on getExternalStickerCache() seems unusual. Verify that String() is the correct method to call for checking if the external sticker cache configuration is not blank.

ruattd · 2024-01-30T08:06:48Z

应该没有什么问题了（至少我没测出什么问题来）

NextAlone · 2024-01-30T09:43:32Z

重新打开以触发 pr check

Co-authored-by: xtaodada <xtao@xtaolink.cn>

feat: external sticker cache

361b21f

sourcery-ai bot reviewed Jan 24, 2024

View reviewed changes

ruattd marked this pull request as draft January 24, 2024 15:28

refactor: feat: external sticker cache

2bee97e

ruattd marked this pull request as ready for review January 30, 2024 07:54

sourcery-ai bot reviewed Jan 30, 2024

View reviewed changes

Merge branch 'dev' into feat-external-sticker-cache

5500352

omg-xtao added the enhancement New feature or request label Jan 30, 2024

omg-xtao assigned ruattd Jan 30, 2024

omg-xtao self-requested a review January 30, 2024 08:30

fix: delete a non-existent invoke in MediaDataController

a102cd6

ruattd force-pushed the feat-external-sticker-cache branch from b3af2ab to a102cd6 Compare January 30, 2024 09:31

NextAlone closed this Jan 30, 2024

NextAlone reopened this Jan 30, 2024

improve

4103863

omg-xtao approved these changes Jan 30, 2024

View reviewed changes

omg-xtao merged commit 97c9309 into NextAlone:dev Jan 30, 2024

omg-xtao referenced this pull request in TeamPGM/Nagram Jan 30, 2024

feat: external sticker cache (omg-xtao#29)

fd92e6a

Co-authored-by: xtaodada <xtao@xtaolink.cn>

ruattd deleted the feat-external-sticker-cache branch January 30, 2024 11:44

LiuYi0526 pushed a commit to LiuYi0526/Nagram that referenced this pull request Jan 30, 2024

feat: external sticker cache (NextAlone#29)

8942240

Co-authored-by: xtaodada <xtao@xtaolink.cn>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: external sticker cache #29

feat: external sticker cache #29

ruattd commented Jan 24, 2024

sourcery-ai bot left a comment

sourcery-ai bot Jan 24, 2024

NextAlone commented Jan 24, 2024

NextAlone commented Jan 24, 2024 •

edited

ruattd commented Jan 24, 2024

sourcery-ai bot left a comment

sourcery-ai bot Jan 30, 2024

sourcery-ai bot Jan 30, 2024

sourcery-ai bot Jan 30, 2024

ruattd commented Jan 30, 2024

NextAlone commented Jan 30, 2024

feat: external sticker cache #29

feat: external sticker cache #29

Conversation

ruattd commented Jan 24, 2024

sourcery-ai bot left a comment

Choose a reason for hiding this comment

sourcery-ai bot Jan 24, 2024

Choose a reason for hiding this comment

NextAlone commented Jan 24, 2024

NextAlone commented Jan 24, 2024 • edited

ruattd commented Jan 24, 2024

sourcery-ai bot left a comment

Choose a reason for hiding this comment

sourcery-ai bot Jan 30, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jan 30, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jan 30, 2024

Choose a reason for hiding this comment

ruattd commented Jan 30, 2024

NextAlone commented Jan 30, 2024

NextAlone commented Jan 24, 2024 •

edited