fix(notifications): add subscription removal#6211
Conversation
WalkthroughIntroduces a new hook-based Telegram subscription flow: adds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant CT as ConnectTelegram
participant Auth as useTgAuthorization
participant Sub as useTgSubscription
participant TG as Telegram
participant CMS as CMS API
Note over CT: Component mount
CT->>Auth: authenticate()
Auth->>TG: getTelegramAuth
TG-->>Auth: TelegramData|null
Auth-->>CT: tgData / isAuthChecked
alt tgData present
CT->>Sub: init(account, Auth)
Sub->>CMS: POST /check-tg-subscription
CMS-->>Sub: { isSubscribed }
Sub-->>CT: isTgSubscribed
else needs authorization
CT-->>U: show "Authorize Telegram"
U->>CT: click Authorize
CT->>Auth: authorize()
Auth->>TG: login flow
TG-->>Auth: success/denied
Auth->>Auth: authenticate()
Auth-->>CT: tgData ready
CT->>Sub: POST /check-tg-subscription
CMS-->>Sub: { isSubscribed }
end
U->>CT: toggle subscription
CT->>Sub: toggleSubscription()
alt adding
Sub->>CMS: POST /add-tg-subscription
CMS-->>Sub: { ok }
Sub-->>CT: isTgSubscribed = true
else removing
Sub->>CMS: POST /remove-tg-subscription
CMS-->>Sub: { ok }
Sub-->>CT: isTgSubscribed = false
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (7)
apps/cowswap-frontend/src/modules/notifications/hooks/useTgAuthorization.ts (2)
75-86: Session polling runs only when tgData exists — consider always polling with page-visibility guard.This misses the “login in another tab” scenario when tgData is null. Polling unconditionally (but skipping when document.hidden) keeps state fresh without excess load.
- useEffect(() => { - if (!tgData) return - - const intervalId = setInterval(authenticate, TG_SESSION_CHECK_INTERVAL) - - return () => { - clearInterval(intervalId) - } - }, [tgData, authenticate]) + useEffect(() => { + const tick = () => { + if (typeof document !== 'undefined' && document.hidden) return + authenticate() + } + const intervalId = setInterval(tick, TG_SESSION_CHECK_INTERVAL) + return () => clearInterval(intervalId) + }, [authenticate])
9-11: Minor: handle NaN in REACT_APP_TG_BOT_ID parsing.If REACT_APP_TG_BOT_ID is set but not numeric, parseInt yields NaN. Consider validating and falling back to the default ID.
Example:
const parsed = Number(process.env.REACT_APP_TG_BOT_ID) const TG_BOT_ID = Number.isFinite(parsed) ? parsed : 7076584722apps/cowswap-frontend/src/modules/notifications/pure/TelegramConnectionStatus/index.tsx (2)
16-22: Expose an optional “isBusy” prop to disable interactions during in-flight CMS/TG calls.Helps prevent double-submits/toggles and aligns with isCmsCallInProgress.
interface TelegramConnectionStatusProps { isLoading: boolean isSubscribed: boolean needsAuthorization: boolean toggleSubscription(): void authorize(): Promise<void> + isBusy?: boolean }
24-30: Pass through new isBusy prop.Keep signature in sync with the interface change.
export function TelegramConnectionStatus({ isLoading, isSubscribed, needsAuthorization, authorize, toggleSubscription, + isBusy, }: TelegramConnectionStatusProps): ReactNode {apps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram.tsx (1)
44-51: Handle script load failures to avoid a stuck loaderAdd an
onerrorhandler soisTelegramScriptLoadingdoesn’t remain true on network/script errors.const scriptElement = document.createElement('script') scriptElement.src = TELEGRAM_AUTH_WIDGET_URL scriptElement.async = true scriptElement.onload = () => { setIsTelegramScriptLoading(false) } + scriptElement.onerror = () => { + setIsTelegramScriptLoading(false) + }apps/cowswap-frontend/src/modules/notifications/hooks/useTgSubscription.ts (2)
22-35: Guard CMS calls and surface errors (optional)
- Early-return from
callSubscriptionApiis fine whenaccountis falsy; consider an early-return intoggleSubscriptiontoo (UX clarity).- Consider adding
.catch()with logging/toast for failures.I can wire a lightweight error reporter (e.g., toast + console) around these calls if you want.
28-33: Idempotency note (optional)If CMS endpoints aren’t idempotent, rapid toggles could cause flapping. Your in-flight guard reduces this; consider server-side idempotency by account+telegram user.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/cowswap-frontend/src/modules/notifications/README.md(1 hunks)apps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram.tsx(3 hunks)apps/cowswap-frontend/src/modules/notifications/hooks/useTgAuthorization.ts(3 hunks)apps/cowswap-frontend/src/modules/notifications/hooks/useTgSubscription.ts(1 hunks)apps/cowswap-frontend/src/modules/notifications/pure/TelegramConnectionStatus/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
apps/cowswap-frontend/src/modules/notifications/hooks/useTgSubscription.ts (2)
apps/cowswap-frontend/src/modules/notifications/hooks/useTgAuthorization.ts (1)
TgAuthorization(27-33)libs/core/src/cms/utils/getCmsClient.ts (1)
getCmsClient(10-17)
apps/cowswap-frontend/src/modules/notifications/hooks/useTgAuthorization.ts (1)
apps/cowswap-frontend/src/modules/notifications/services/getTelegramAuth.ts (1)
getTelegramAuth(15-40)
apps/cowswap-frontend/src/modules/notifications/pure/TelegramConnectionStatus/index.tsx (3)
libs/ui/src/pure/Button/index.tsx (1)
FancyButton(184-195)libs/ui/src/pure/Loader/index.tsx (1)
Loader(32-43)apps/cowswap-frontend/src/legacy/components/Toggle/index.tsx (1)
Toggle(107-132)
apps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram.tsx (3)
apps/cowswap-frontend/src/modules/notifications/hooks/useTgAuthorization.ts (1)
useTgAuthorization(35-88)apps/cowswap-frontend/src/modules/notifications/hooks/useTgSubscription.ts (1)
useTgSubscription(15-86)apps/cowswap-frontend/src/modules/notifications/pure/TelegramConnectionStatus/index.tsx (1)
TelegramConnectionStatus(24-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Setup
- GitHub Check: Cypress
🔇 Additional comments (5)
apps/cowswap-frontend/src/modules/notifications/README.md (1)
10-10: Add “removeSubscription” permission — good catch.This aligns the CMS role with the new unsubscribe flow.
Please confirm in the CMS (dev) that the three endpoints are actually exposed under:
- POST /add-tg-subscription
- POST /remove-tg-subscription
- GET /check-tg-subscription
If the slugs differ, the hook calls will fail with 403/404.
apps/cowswap-frontend/src/modules/notifications/hooks/useTgAuthorization.ts (2)
40-50: authenticate() promise wrapper — LGTM.Resolves on both success and failure paths from getTelegramAuth; state updates are consistent.
29-33: API change validated; no further action requiredConnectTelegram is the sole consumer—it invokes authenticate() on mount and passes authorize() into TelegramConnectionStatus, leveraging isAuthChecked and isLoginInProgress for loading state.
apps/cowswap-frontend/src/modules/notifications/pure/TelegramConnectionStatus/index.tsx (1)
9-14: Styling looks consistent with FancyButton; good.Padding and cursor improve UX.
apps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram.tsx (1)
68-74: Prop wiring looks correct
TelegramConnectionStatusreceives the right flags and handlers, matching the new auth/subscription split.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/cowswap-frontend/src/modules/notifications/pure/TelegramConnectionStatus/index.cosmos.tsx (3)
3-4: Align handler name with prop and add useful no-op logsKeeps fixture intent clear and makes clicks visible in Cosmos.
-const subscribeAccount = (): void => {} -const authorize = (): Promise<void> => Promise.resolve() +const toggleSubscription = (): void => { + console.info('[cosmos] toggleSubscription clicked') +} +const authorize = async (): Promise<void> => { + console.info('[cosmos] authorize clicked') +}
26-33: Rename fixture for clarity“needLogin” is misleading now that authorization is explicit. Prefer “unsubscribed”.
Update key name (illustrative):
const Fixtures = { // ... unsubscribed: () => ( <TelegramConnectionStatus isLoading={false} isSubscribed={false} needsAuthorization={false} authorize={authorize} toggleSubscription={toggleSubscription} /> ), }Also, if you adopt the handler rename above, switch the prop value here to
toggleSubscription.- toggleSubscription={subscribeAccount} + toggleSubscription={toggleSubscription}
34-41: LGTM: needsAuthorization fixtureCovers auth CTA path. If you rename the handler, adjust prop value accordingly.
- toggleSubscription={subscribeAccount} + toggleSubscription={toggleSubscription}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/cowswap-frontend/src/modules/notifications/pure/TelegramConnectionStatus/index.cosmos.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/cowswap-frontend/src/modules/notifications/pure/TelegramConnectionStatus/index.cosmos.tsx (1)
apps/cowswap-frontend/src/modules/notifications/pure/TelegramConnectionStatus/index.tsx (1)
TelegramConnectionStatus(24-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Setup
- GitHub Check: Cypress
🔇 Additional comments (2)
apps/cowswap-frontend/src/modules/notifications/pure/TelegramConnectionStatus/index.cosmos.tsx (2)
7-15: LGTM: loading state fixtureProps match component contract; loader path covered.
17-23: LGTM: subscribed state fixtureCovers active toggle path.
2 participants
Summary
Needs this PR to be merged first: cowprotocol/cms#65
In this PR I added TG unsubscribe functionality.
Now you the toggle off the button and it will unsubscribe you.
Before you perform subscribe/unsubscribe you have to be authorized in Telegram.
We periodically check TG authorization and if there is no active session we display "Authorize Telegram" button.
To Test
Can be tested only on dev due to TG domain restrictions.
Summary by CodeRabbit
New Features
Refactor
Documentation