-
Notifications
You must be signed in to change notification settings - Fork 112
chore: hide share pro plan #171
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 GuideThis PR changes how subscription plans and upgrade prompts are handled based on whether the app is running on an official host, defaulting users to Pro and hiding upgrade prompts on non-official hosts, and adjusts the list of official hostnames. Sequence diagram for subscription handling in SharePanelsequenceDiagram
actor User
participant SharePanel
participant SubscriptionUtils as SubscriptionUtils_isOfficialHost
participant AppHandlers as AppHandlers_getSubscriptions
User->>SharePanel: Open_share_panel(viewId)
activate SharePanel
SharePanel->>SubscriptionUtils: isOfficialHost()
alt Not_official_host
SubscriptionUtils-->>SharePanel: false
SharePanel->>SharePanel: setActiveSubscriptionPlan(SubscriptionPlan.Pro)
else Official_host
SubscriptionUtils-->>SharePanel: true
SharePanel->>AppHandlers: getSubscriptions()
AppHandlers-->>SharePanel: Subscription_list
SharePanel->>SharePanel: derive_activeSubscriptionPlan_from_subscriptions
end
deactivate SharePanel
Note over User,SharePanel: UpgradeBanner is rendered only when isOfficialHost() is true
Sequence diagram for subscription handling in PublishManage and InviteMembersequenceDiagram
actor User
participant PublishManage
participant InviteMember
participant SubscriptionUtils as SubscriptionUtils_isOfficialHost
participant AppHandlers as AppHandlers_getSubscriptions
User->>PublishManage: Open_publish_manage_modal()
activate PublishManage
PublishManage->>SubscriptionUtils: isOfficialHost()
alt Not_official_host
SubscriptionUtils-->>PublishManage: false
PublishManage->>PublishManage: setActiveSubscription(SubscriptionPlan.Pro)
else Official_host
SubscriptionUtils-->>PublishManage: true
PublishManage->>AppHandlers: getSubscriptions()
AppHandlers-->>PublishManage: Subscription_list
PublishManage->>PublishManage: derive_activeSubscription_from_subscriptions
end
deactivate PublishManage
User->>InviteMember: Open_invite_member_modal()
activate InviteMember
InviteMember->>SubscriptionUtils: isOfficialHost()
alt Not_official_host
SubscriptionUtils-->>InviteMember: false
InviteMember->>InviteMember: setActiveSubscriptionPlan(SubscriptionPlan.Pro)
else Official_host
SubscriptionUtils-->>InviteMember: true
InviteMember->>AppHandlers: getSubscriptions()
AppHandlers-->>InviteMember: Subscription_list
InviteMember->>InviteMember: derive_activeSubscriptionPlan_from_subscriptions
end
deactivate InviteMember
Updated class diagram for subscription-related components and utilitiesclassDiagram
class SubscriptionPlan {
<<enumeration>>
Free
Pro
}
class SubscriptionUtils {
+OFFICIAL_HOSTNAMES Set~string~
+isOfficialHost() bool
+getBaseUrlHostname() string
}
class SharePanel {
+viewId string
+activeSubscriptionPlan SubscriptionPlan
+loadSubscription() void
}
class PublishManage {
+onClose() void
+activeSubscription SubscriptionPlan
+loadSubscription() void
}
class InviteMember {
+workspace any
+activeSubscriptionPlan SubscriptionPlan
+loadSubscription() void
}
class AppHandlers {
+getSubscriptions() SubscriptionPlan[]
}
SharePanel --> SubscriptionPlan
PublishManage --> SubscriptionPlan
InviteMember --> SubscriptionPlan
SharePanel ..> SubscriptionUtils : uses
PublishManage ..> SubscriptionUtils : uses
InviteMember ..> SubscriptionUtils : uses
SharePanel ..> AppHandlers : uses
PublishManage ..> AppHandlers : uses
InviteMember ..> AppHandlers : uses
SubscriptionUtils o-- SubscriptionPlan : OFFICIAL_HOSTNAMES_drives_isOfficialHost
Flow diagram for isOfficialHost and subscription behaviorflowchart TD
A[Start] --> B[Get_base_url_hostname_from_APPFLOWY_BASE_URL]
B --> C{Hostname_in_OFFICIAL_HOSTNAMES?}
C -->|Yes| D[isOfficialHost returns true]
C -->|No| E[isOfficialHost returns false]
D --> F{Component: SharePanel_or_PublishManage_or_InviteMember}
E --> F
F --> G{isOfficialHost_result}
G -->|true| H[Call_getSubscriptions_and_compute_active_subscription]
G -->|false| I[Set_active_subscription_to_SubscriptionPlan.Pro]
H --> J[On_official_hosts_show_UpgradeBanner_in_SharePanel]
I --> K[On_non_official_hosts_hide_UpgradeBanner_and_treat_as_Pro]
J --> L[End]
K --> L[End]
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:
- In the components where you branch on isOfficialHost (SharePanel, PublishManage, InviteMember), consider computing this once (e.g., const isOfficial = isOfficialHost()) and reusing it in both effects and JSX to avoid repeated calls and ensure consistent behavior if the hosting logic ever changes.
- Changing OFFICIAL_HOSTNAMES from 'localhost:8000' to 'localhost' may no longer match configurations where APPFLOWY_BASE_URL includes an explicit port (e.g., http://localhost:8000); if you intend to treat any localhost with any port as official, consider normalizing the hostname or matching on hostname only instead of the full host:port string.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the components where you branch on isOfficialHost (SharePanel, PublishManage, InviteMember), consider computing this once (e.g., const isOfficial = isOfficialHost()) and reusing it in both effects and JSX to avoid repeated calls and ensure consistent behavior if the hosting logic ever changes.
- Changing OFFICIAL_HOSTNAMES from 'localhost:8000' to 'localhost' may no longer match configurations where APPFLOWY_BASE_URL includes an explicit port (e.g., http://localhost:8000); if you intend to treat any localhost with any port as official, consider normalizing the hostname or matching on hostname only instead of the full host:port string.
## Individual Comments
### Comment 1
<location> `src/utils/subscription.ts:9` </location>
<code_context>
import { getConfigValue } from '@/utils/runtime-config';
-const OFFICIAL_HOSTNAMES = new Set(['beta.appflowy.cloud', 'test.appflowy.cloud', 'localhost:8000']);
+const OFFICIAL_HOSTNAMES = new Set(['beta.appflowy.cloud', 'test.appflowy.cloud', 'localhost']);
function getBaseUrlHostname(): string | null {
</code_context>
<issue_to_address>
**question:** Revisit official-host detection to account for additional local addresses or ports if needed.
Since `URL.hostname` excludes the port, this change now matches purely on hostnames. Please confirm whether other local hosts (e.g. `127.0.0.1` or custom dev hostnames) should also be treated as official; if so, they’ll need to be added to `OFFICIAL_HOSTNAMES` or handled elsewhere.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| import { getConfigValue } from '@/utils/runtime-config'; | ||
|
|
||
| const OFFICIAL_HOSTNAMES = new Set(['beta.appflowy.cloud', 'test.appflowy.cloud', 'localhost:8000']); | ||
| const OFFICIAL_HOSTNAMES = new Set(['beta.appflowy.cloud', 'test.appflowy.cloud', 'localhost']); |
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.
question: Revisit official-host detection to account for additional local addresses or ports if needed.
Since URL.hostname excludes the port, this change now matches purely on hostnames. Please confirm whether other local hosts (e.g. 127.0.0.1 or custom dev hostnames) should also be treated as official; if so, they’ll need to be added to OFFICIAL_HOSTNAMES or handled elsewhere.
Description
Checklist
General
Testing
Feature-Specific
Summary by Sourcery
Adjust subscription handling and visibility of upgrade prompts based on whether the app is running on an official host.
Bug Fixes:
Enhancements: