-
Notifications
You must be signed in to change notification settings - Fork 131
Disable Gifts in Profile #54
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
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideIntroduces a new disableGifts configuration flag that, when enabled, suppresses gift counts and the Gifts tab across profile and shared media views. The flag is added to the main config, gated in multiple UI components, and exposed in the settings with accompanying string resources. Class diagram for NaConfig and disableGifts flagclassDiagram
class NaConfig {
+disableGifts: ConfigFlag
+addConfig(name, parentFlag, id, defaultValue)
}
class ConfigFlag {
+Bool(): boolean
}
NaConfig --> ConfigFlag: has
NaConfig : +disableGifts = addConfig("DisableGifts", disableTrendingFlags, 16, false)
Class diagram for ProfileGiftsContainer and Gifts tab logicclassDiagram
class ProfileGiftsContainer {
-fragment: BaseFragment
-getGiftsCount(): int
}
class NaConfig {
+getDisableGifts(): ConfigFlag
}
ProfileGiftsContainer --> NaConfig: uses
ProfileGiftsContainer : getGiftsCount() checks NaConfig.disableGifts.Bool()
Class diagram for SharedMediaLayout and Gifts tab logicclassDiagram
class SharedMediaLayout {
-initialTab: int
-updateTabs(animated): void
}
class NaConfig {
+getDisableGifts(): ConfigFlag
}
SharedMediaLayout --> NaConfig: uses
SharedMediaLayout : initialTab logic checks NaConfig.disableGifts.Bool()
SharedMediaLayout : updateTabs() checks NaConfig.disableGifts.Bool()
Class diagram for ProfileActivity and Gifts tab logicclassDiagram
class ProfileActivity {
-getView(dialogId): SourceView
-updateRowsIds(): void
}
class NaConfig {
+getDisableGifts(): ConfigFlag
}
ProfileActivity --> NaConfig: uses
ProfileActivity : getView() and updateRowsIds() check NaConfig.disableGifts.Bool()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Extract the repeated NaConfig.INSTANCE.getDisableGifts().Bool() checks into a single helper or utility method to avoid duplication and improve readability across ProfileGiftsContainer, SharedMediaLayout, and ProfileActivity.
- Consider moving the DisableGifts toggle out of the 'Disable Trending' group into a dedicated ‘Profile Features’ or similar section so it’s more discoverable and semantically organized in the settings UI.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Extract the repeated NaConfig.INSTANCE.getDisableGifts().Bool() checks into a single helper or utility method to avoid duplication and improve readability across ProfileGiftsContainer, SharedMediaLayout, and ProfileActivity.
- Consider moving the DisableGifts toggle out of the 'Disable Trending' group into a dedicated ‘Profile Features’ or similar section so it’s more discoverable and semantically organized in the settings UI.
## Individual Comments
### Comment 1
<location> `TMessagesProj/src/main/java/org/telegram/ui/Components/SharedMediaLayout.java:1519` </location>
<code_context>
} else if (!NaConfig.INSTANCE.getDisableStories().Bool() && (userInfo != null && userInfo.stories_pinned_available || chatInfo != null && chatInfo.stories_pinned_available || isStoriesView())) {
this.initialTab = getInitialTab();
- } else if (userInfo != null && userInfo.stargifts_count > 0 || chatInfo != null && chatInfo.stargifts_count > 0) {
+ } else if (!NaConfig.INSTANCE.getDisableGifts().Bool() && (userInfo != null && userInfo.stargifts_count > 0 || chatInfo != null && chatInfo.stargifts_count > 0)) {
this.initialTab = TAB_GIFTS;
} else if (initialTab != -1 && topicId == 0) {
</code_context>
<issue_to_address>
**suggestion:** The conditional disables the gifts tab based on the config, but the logic could be clearer with parentheses.
Adding parentheses will make the condition's logic and operator precedence explicit, reducing the risk of misinterpretation.
Suggested implementation:
```java
} else if (!NaConfig.INSTANCE.getDisableGifts().Bool() && ((userInfo != null && userInfo.stargifts_count > 0) || (chatInfo != null && chatInfo.stargifts_count > 0))) {
```
```java
boolean hasGifts = !NaConfig.INSTANCE.getDisableGifts().Bool() && giftsContainer != null && ((userInfo != null && userInfo.stargifts_count > 0) || (info != null && info.stargifts_count > 0));
```
</issue_to_address>
### Comment 2
<location> `TMessagesProj/src/main/java/org/telegram/ui/Components/SharedMediaLayout.java:6497` </location>
<code_context>
boolean hasStories = (DialogObject.isUserDialog(dialog_id) || DialogObject.isChatDialog(dialog_id)) && !DialogObject.isEncryptedDialog(dialog_id) && (userInfo != null && userInfo.stories_pinned_available || info != null && info.stories_pinned_available || isStoriesView()) && includeStories();
hasStories = !NaConfig.INSTANCE.getDisableStories().Bool() && hasStories;
- boolean hasGifts = giftsContainer != null && (userInfo != null && userInfo.stargifts_count > 0 || info != null && info.stargifts_count > 0);
+ boolean hasGifts = !NaConfig.INSTANCE.getDisableGifts().Bool() && giftsContainer != null && (userInfo != null && userInfo.stargifts_count > 0 || info != null && info.stargifts_count > 0);
int changed = 0;
if ((hasStories || hasBotPreviews) != scrollSlidingTextTabStrip.hasTab(TAB_STORIES)) {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Logical operator precedence may cause unintended evaluation order.
Please add parentheses to clarify the intended grouping of conditions and prevent logical errors.
```suggestion
boolean hasGifts = !NaConfig.INSTANCE.getDisableGifts().Bool()
&& giftsContainer != null
&& ((userInfo != null && userInfo.stargifts_count > 0)
|| (info != null && info.stargifts_count > 0));
```
</issue_to_address>
### Comment 3
<location> `TMessagesProj/src/main/java/org/telegram/ui/ProfileActivity.java:3579` </location>
<code_context>
if (openCommonChats) {
initialTab = SharedMediaLayout.TAB_COMMON_GROUPS;
- } else if (openGifts && (userInfo != null && userInfo.stargifts_count > 0 || chatInfo != null && chatInfo.stargifts_count > 0)) {
+ } else if (!NaConfig.INSTANCE.getDisableGifts().Bool() && openGifts && (userInfo != null && userInfo.stargifts_count > 0 || chatInfo != null && chatInfo.stargifts_count > 0)) {
initialTab = SharedMediaLayout.TAB_GIFTS;
openedGifts = true;
</code_context>
<issue_to_address>
**suggestion:** Operator precedence in the conditional could be clarified with parentheses.
Group the `userInfo` and `chatInfo` conditions with parentheses to ensure the intended logic is clear and prevent misinterpretation.
Suggested implementation:
```java
} else if (!NaConfig.INSTANCE.getDisableGifts().Bool() && openGifts && ((userInfo != null && userInfo.stargifts_count > 0) || (chatInfo != null && chatInfo.stargifts_count > 0))) {
```
```java
if (!NaConfig.INSTANCE.getDisableGifts().Bool() && !hasMedia && ((userInfo != null && userInfo.stargifts_count > 0) || (chatInfo != null && chatInfo.stargifts_count > 0))) {
```
</issue_to_address>
### Comment 4
<location> `TMessagesProj/src/main/java/org/telegram/ui/ProfileActivity.java:9865` </location>
<code_context>
hasMedia = userInfo.bot_info.has_preview_medias;
}
- if (!hasMedia && (userInfo != null && userInfo.stargifts_count > 0 || chatInfo != null && chatInfo.stargifts_count > 0)) {
+ if (!NaConfig.INSTANCE.getDisableGifts().Bool() && !hasMedia && (userInfo != null && userInfo.stargifts_count > 0 || chatInfo != null && chatInfo.stargifts_count > 0)) {
hasMedia = true;
}
</code_context>
<issue_to_address>
**suggestion:** Logical grouping should be clarified for maintainability.
Add parentheses to clarify the logical grouping and prevent ambiguity in condition evaluation.
Suggested implementation:
```java
} else if (!NaConfig.INSTANCE.getDisableGifts().Bool() && openGifts && ((userInfo != null && userInfo.stargifts_count > 0) || (chatInfo != null && chatInfo.stargifts_count > 0))) {
```
```java
if (!NaConfig.INSTANCE.getDisableGifts().Bool() && !hasMedia && ((userInfo != null && userInfo.stargifts_count > 0) || (chatInfo != null && chatInfo.stargifts_count > 0))) {
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
TMessagesProj/src/main/java/org/telegram/ui/Components/SharedMediaLayout.java
Show resolved
Hide resolved
TMessagesProj/src/main/java/org/telegram/ui/Components/SharedMediaLayout.java
Show resolved
Hide resolved
|
@sourcery-ai[bot] xtao 看看,看起来没啥问题 |
TMessagesProj/src/main/java/org/telegram/ui/Gifts/ProfileGiftsContainer.java
Show resolved
Hide resolved
Refactor gift count retrieval logic to remove NaConfig check.
这会禁用个人页面中的礼物 Tab
由于我的垃圾思维(指返回所有stargifts_count为0),这也有可能不止关闭个人页面中的礼物功能,所以请开发者小心处理
选项会在“禁用趋势推荐”中
Summary by Sourcery
Introduce a configuration toggle to disable the Gifts feature on profile and shared media UIs and hook it into the settings screen.
New Features:
DisableGiftsconfig flag in NaConfigEnhancements:
Documentation: