feat: implement community inactive status validation system #697
Conversation
This commit introduces comprehensive validation to prevent operations on
inactive communities while preserving administrative access for reactivation.
Key Changes:
- Add 'active' field validation to all community lookup operations
- Implement automatic cleanup of inactive default communities from user settings
- Add 'community_inactive' i18n message in all 10 supported languages
- Update community search functions to filter out inactive communities
- Preserve admin access for community creators to manage inactive communities
- Add active status checks to order channel resolution and fee calculations
- Update validation functions to handle inactive community scenarios
Technical Implementation:
- Modified Community queries to include 'active: true' filter where appropriate
- Updated getCommunityInfo() helper for optimized active community lookup
- Enhanced user settings to auto-clear inactive default communities
- Maintained backward compatibility with existing admin functionality
- Added comprehensive error handling with localized messages
Impact:
- Users cannot create/take orders in inactive communities
- Users cannot set inactive communities as default
- Community creators retain full admin access for reactivation
- System automatically handles inactive community cleanup
- Improved user experience with clear error messaging
Files Modified:
- Community validation: util/communityHelper.ts, bot/validations.ts
- Community management: bot/modules/community/{actions,commands}.ts
- User settings: bot/modules/user/scenes/settings.ts
- Utility functions: util/index.ts
- Internationalization: locales/*.yaml (10 languages)
- Job systems: jobs/{check_solvers,communities}.ts
- Models: models/community.ts
WalkthroughIntroduces an active flag on Community, changes deletion to deactivation across jobs, restricts community lookups to active ones, clears inactive default_community_id from users, updates validations and helpers to respect active state, adjusts scheduler/imports to call disableCommunity, and adds a community_inactive translation key in multiple locales. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Pre-merge checks (3 passed)✅ Passed checks (3 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
models/community.ts (1)
48-53: Critical: backfill missingactivefield
- Run a one-off migration before deploying any code that filters on
{ active: true }(usedb.communities.updateMany({ active: { $exists: false } }, { $set: { active: true } })) to prevent all pre-existing docs from being treated as inactive.- (Optional) During rollout, use
{ active: { $ne: false } }in filters until every instance has run the migration.- (Optional) Add a compound index on
{ group: 1, active: 1 }to speed up group-and-active queries.- Update any tests or fixtures that create or stub
Communitydocuments to include the newactiveflag.jobs/check_solvers.ts (1)
12-20: Harden deactivation flow: skip inactive, default counters, and guard solvers array
- Filter only active communities to avoid warning/deactivation loops on already inactive ones.
- Default warning_messages_count to 0 to prevent NaN.
- Guard when solvers is undefined/non-array.
Apply:
- const communities = await Community.find(); + const communities = await Community.find({ active: true }); - if (community.solvers.length > 0) { - continue; - } else { - await notifyAdmin(community, bot); - } + if (Array.isArray(community.solvers) && community.solvers.length > 0) continue; + await notifyAdmin(community, bot); - community.warning_messages_count += 1; + community.warning_messages_count = (community.warning_messages_count ?? 0) + 1;Also applies to: 31-41, 46-56
jobs/communities.ts (1)
3-3: Critical: wrong chat identifier and stale log message; also skip already inactive and validate TTL
- creator_id appears to be a Mongo User _id, not a Telegram chat id. Using bot.telegram.getChat(community.creator_id) will fail. Load the User and send to user.tg_id.
- Log says “being deleted” but action is deactivation.
- Filter only active communities and validate COMMUNITY_TTL.
- Prefer countDocuments over deprecated count.
Apply:
-import { Order, Community } from '../models'; +import { Order, Community, User } from '../models'; -const disableCommunity = async (bot: Telegraf<CommunityContext>) => { +const disableCommunity = async (bot: Telegraf<CommunityContext>) => { try { - const communities = await Community.find(); + const ttlDays = Number(process.env.COMMUNITY_TTL); + if (!Number.isFinite(ttlDays) || ttlDays <= 0) { + logger.error('Invalid COMMUNITY_TTL; skipping disableCommunity run'); + return; + } + const communities = await Community.find({ active: true }); for (const community of communities) { // Delete communities with COMMUNITY_TTL days without a successful order - const days = 86400 * Number(process.env.COMMUNITY_TTL); + const days = 86400 * ttlDays; const time = new Date(); time.setSeconds(time.getSeconds() - days); // If is a new community we don't do anything if (community.created_at > time) { continue; } - const orders = await Order.count({ + const orders = await Order.countDocuments({ created_at: { $gte: time }, status: 'SUCCESS', community_id: community.id, }); if (orders == 0) { logger.info( - `Community: ${community.name} have ${process.env.COMMUNITY_TTL} days without a successfully completed order, it's being deleted!`, + `Community: ${community.name} has ${process.env.COMMUNITY_TTL} days without a successfully completed order, it's being deactivated!`, ); community.active = false; await community.save(); - // Notify the community admin - const admin = await bot.telegram.getChat(community.creator_id); - if (admin) { - await bot.telegram.sendMessage( - admin.id, - `Your community ${community.name} has been deactivated due to inactivity. Please contact support if you have any questions.`, - ); - } + // Notify the community admin + try { + const adminUser = await User.findById(community.creator_id); + if (adminUser?.tg_id) { + await bot.telegram.sendMessage( + adminUser.tg_id, + `Your community ${community.name} has been deactivated due to inactivity.`, + ); + } + } catch (e) { + logger.warn(`Failed to notify admin about deactivation for ${community.name}: ${String(e)}`); + } } } } catch (error) { const message = String(error); - logger.error(`disableCommunity catch error: ${message}`); + logger.error(`disableCommunity catch error: ${message}`); } }; export default disableCommunity;Also applies to: 7-7, 25-37, 42-42, 46-46
bot/modules/community/commands.ts (1)
220-229: Deactivate communities instead of removing them
In bot/modules/community/commands.ts (lines 220–229), replace the hard delete with a soft-delete:- await community.remove(); + community.active = false; + await community.save();Also update the command label in the UI to “Disable community” for consistency.
util/index.ts (1)
298-307: Early return skips deletion; fallback to default channel instead.deleteOrderFromChannel returns early when no active community, leaving stale channel messages. Keep fallback to process.env.CHANNEL and proceed to delete.
if (order.community_id) { - const community = await Community.findOne({ _id: order.community_id, active: true }); - if (!community) { - return channel; - } - if (community.order_channels.length === 1) { - channel = community.order_channels[0].name; - } else { - for await (const c of community.order_channels) { - if (c.type === order.type) { - channel = c.name; - } - } - } + const community = await Community.findOne({ _id: order.community_id, active: true }); + if (community) { + if (community.order_channels.length === 1) { + channel = community.order_channels[0].name; + } else { + for (const c of community.order_channels) { + if (c.type === order.type) { + channel = c.name; + break; + } + } + } + } } await telegram.deleteMessage(channel!, Number(order.tg_channel_message1!));Also remove unnecessary async iteration.
🧹 Nitpick comments (8)
jobs/check_solvers.ts (1)
33-41: Compute warnings after validating env and consider notifying admin on deactivation
- Move increment after MAX_ADMIN_WARNINGS_BEFORE_DEACTIVATION validation to avoid mutating state when env is invalid.
- Optional: send a localized deactivation notice to the creator when the community is deactivated here (mirroring jobs/communities.ts).
- community.warning_messages_count = (community.warning_messages_count ?? 0) + 1; - - const maxWarningsCount = Number.parseInt( + const maxWarningsCount = Number.parseInt( process.env.MAX_ADMIN_WARNINGS_BEFORE_DEACTIVATION ?? '', ); if (!Number.isFinite(maxWarningsCount) || maxWarningsCount < 0) { logger.error( 'Invalid MAX_ADMIN_WARNINGS_BEFORE_DEACTIVATION; skipping notifyAdmin', ); return; } + community.warning_messages_count = (community.warning_messages_count ?? 0) + 1;If you want, I can add a new i18n key like community_deactivated for this path.
Also applies to: 46-54
locales/fa.yaml (1)
471-471: FA i18n: unify “Community” termThe file mixes «کامیونیتی» and «انجمن». For consistency (many keys use «انجمن»), consider:
-community_inactive: این کامیونیتی در حال حاضر غیرفعال است و نمیتواند برای معاملات استفاده شود +community_inactive: این انجمن در حال حاضر غیرفعال است و نمیتواند برای معاملات استفاده شودbot/modules/community/commands.ts (1)
244-256: Visibility toggle on inactive communitiesToggling public on inactive communities may be confusing. Optional: block this when active === false or show a warning.
bot/start.ts (1)
225-227: Guard scheduled disableCommunity with try/catch for resilience.Avoid unhandled rejections in the daily job.
schedule.scheduleJob(`33 0 * * *`, async () => { - await disableCommunity(bot); + try { + await disableCommunity(bot); + } catch (error) { + logger.error(error); + } });bot/modules/community/actions.ts (2)
64-66: Block info for inactive communities — consider creator bypass.Current behavior stops everyone. If creators should still view details to manage/reactivate, gate by creator_id instead of a blanket block.
Would you like me to add a creator bypass here (allow info for creator, block others)?
117-123: Fix Prettier errors and avoid any-cast for tgId.Formatting is failing CI (lines 114–126). Also prefer ctx.from?.id over (ctx.update as any).
- const tgId = (ctx.update as any).callback_query.from.id; + const tgId = ctx.from?.id; - // Check if the community is active before setting it as default - const community = await Community.findById(defaultCommunityId); - if (!community || !community.active) { - return await ctx.reply(ctx.i18n.t('community_inactive')); - } + // Check if the community is active before setting it as default + const community = await Community.findById(defaultCommunityId); + if (!community || !community.active) { + return await ctx.reply(ctx.i18n.t('community_inactive')); + }Run: npm run format
util/communityHelper.ts (1)
39-45: Clear stale defaults when inactive — good safeguard.Prevents ghost defaults and downstream bans/fee lookups on inactive communities.
If types allow, normalize communityId to String(community._id) to avoid ObjectId/string drift.
util/index.ts (1)
324-333: Prefer deterministic loop; avoid async forEach.forEach(async ...) isn’t awaited. Use for..of and break on match.
- } else { - community.order_channels.forEach(async (c: IOrderChannel) => { - if (c.type === order.type) { - channel = c.name; - } - }); - } + } else { + for (const c of community.order_channels) { + if (c.type === order.type) { + channel = c.name; + break; + } + } + }Run: npm run format
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
bot/modules/community/actions.ts(3 hunks)bot/modules/community/commands.ts(6 hunks)bot/modules/user/scenes/settings.ts(1 hunks)bot/start.ts(2 hunks)bot/validations.ts(2 hunks)jobs/check_solvers.ts(1 hunks)jobs/communities.ts(2 hunks)jobs/index.ts(2 hunks)locales/de.yaml(1 hunks)locales/en.yaml(1 hunks)locales/es.yaml(1 hunks)locales/fa.yaml(1 hunks)locales/fr.yaml(1 hunks)locales/it.yaml(1 hunks)locales/ko.yaml(1 hunks)locales/pt.yaml(1 hunks)locales/ru.yaml(1 hunks)locales/uk.yaml(1 hunks)models/community.ts(2 hunks)util/communityHelper.ts(2 hunks)util/index.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
jobs/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
jobs/**/*.ts: Implement background jobs for pending payments, order expiration/cleanup, community earnings, node health, and solver availability
Use the specified scheduling intervals: pending payments every 5m (PENDING_PAYMENT_WINDOW), order cancellation every 20s, order deletion hourly at :25, community earnings every 10m, node health every 1m, solver availability daily at midnight
Files:
jobs/check_solvers.tsjobs/index.tsjobs/communities.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use Winston for logging (avoid console) and include contextual details and timeout monitoring where relevant
Wrap risky operations in try-catch with meaningful error context
Files:
jobs/check_solvers.tsutil/communityHelper.tsutil/index.tsbot/modules/user/scenes/settings.tsjobs/index.tsbot/validations.tsjobs/communities.tsbot/modules/community/actions.tsbot/modules/community/commands.tsbot/start.tsmodels/community.ts
locales/**/*.{yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Store i18n message templates as YAML files under locales/ for multi-language support
Files:
locales/fr.yamllocales/pt.yamllocales/fa.yamllocales/ko.yamllocales/it.yamllocales/uk.yamllocales/es.yamllocales/ru.yamllocales/de.yamllocales/en.yaml
util/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared utilities and helpers under util/
Files:
util/communityHelper.tsutil/index.ts
bot/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
bot/**/*.ts: Use custom Telegraf context types (MainContext, CommunityContext) in handlers and modules
Integrate @grammyjs/i18n for dynamic language selection based on user preferences
Files:
bot/modules/user/scenes/settings.tsbot/validations.tsbot/modules/community/actions.tsbot/modules/community/commands.tsbot/start.ts
bot/modules/*/{commands,actions,messages,scenes,index}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Structure each feature module with commands.ts, actions.ts, messages.ts, scenes.ts, and index.ts
Files:
bot/modules/community/actions.tsbot/modules/community/commands.ts
bot/start.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Implement bot initialization, command registration, and scheduled job setup in bot/start.ts
Files:
bot/start.ts
models/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Define MongoDB schemas with comprehensive TypeScript interfaces for all models
Files:
models/community.ts
🧠 Learnings (4)
📚 Learning: 2025-05-02T19:37:57.466Z
Learnt from: Catrya
PR: lnp2pBot/bot#650
File: bot/modules/nostr/events.ts:43-54
Timestamp: 2025-05-02T19:37:57.466Z
Learning: In the lnp2pBot codebase, communities can have at most 2 order channels - it's not possible for community.order_channels.length to be greater than 2.
Applied to files:
util/index.ts
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
PR: lnp2pBot/bot#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to jobs/**/*.ts : Implement background jobs for pending payments, order expiration/cleanup, community earnings, node health, and solver availability
Applied to files:
jobs/index.tsbot/start.ts
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
PR: lnp2pBot/bot#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to bot/start.ts : Implement bot initialization, command registration, and scheduled job setup in bot/start.ts
Applied to files:
bot/start.ts
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
PR: lnp2pBot/bot#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to jobs/**/*.ts : Use the specified scheduling intervals: pending payments every 5m (PENDING_PAYMENT_WINDOW), order cancellation every 20s, order deletion hourly at :25, community earnings every 10m, node health every 1m, solver availability daily at midnight
Applied to files:
bot/start.ts
🧬 Code graph analysis (9)
util/communityHelper.ts (2)
models/index.ts (1)
Community(9-9)bot/modules/community/messages.ts (2)
community(125-140)ctx(47-117)
locales/it.yaml (1)
bot/modules/community/messages.ts (1)
community(125-140)
util/index.ts (1)
jobs/calculate_community_earnings.ts (1)
orders(4-43)
bot/validations.ts (2)
models/index.ts (1)
Community(9-9)tests/bot/validation.spec.ts (7)
newCommunity(427-444)newCommunity(399-418)Community(1122-1126)Community(1128-1132)user(1134-1143)beforeEach(1104-1144)community(1105-1111)
jobs/communities.ts (2)
jobs/index.ts (1)
disableCommunity(18-18)bot/modules/community/communityContext.ts (1)
CommunityContext(18-22)
bot/modules/community/actions.ts (2)
bot/modules/community/scenes.ts (4)
ctx(842-855)ctx(856-898)ctx(538-552)ctx(748-761)bot/modules/community/messages.ts (1)
ctx(147-186)
bot/modules/community/commands.ts (2)
models/index.ts (1)
Community(9-9)bot/modules/community/index.ts (1)
ctx(37-39)
bot/start.ts (2)
jobs/index.ts (1)
disableCommunity(18-18)bot/modules/community/index.ts (3)
bot(13-93)ctx(31-33)ctx(28-30)
models/community.ts (2)
tests/bot/validation.spec.ts (2)
community(1105-1111)newCommunity(399-418)bot/modules/community/communityContext.ts (2)
CommunityWizardState(27-55)CommunityContext(18-22)
🪛 GitHub Actions: NodeJS Continuous Integration
util/index.ts
[error] 298-307: Prettier formatting did not match. Run 'npm run format' to fix code style issues.
[error] 324-333: Prettier formatting did not match. Run 'npm run format' to fix code style issues.
[error] 348-357: Prettier formatting did not match. Run 'npm run format' to fix code style issues.
bot/validations.ts
[error] 737-746: Prettier formatting did not match. Run 'npm run format' to fix code style issues.
bot/modules/community/actions.ts
[error] 114-126: Prettier formatting did not match. Run 'npm run format' to fix code style issues.
⏰ 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: ci_to_main
- GitHub Check: Analyze (javascript)
🔇 Additional comments (19)
locales/fr.yaml (1)
470-470: LGTM: added community_inactive key is clear and consistent with surrounding messages.
No punctuation change needed to match neighboring entries.locales/uk.yaml (1)
468-468: LGTM: Ukrainian translation reads naturally and matches intent.locales/es.yaml (1)
472-472: LGTM: Spanish copy is clear and aligns with usage.locales/ko.yaml (1)
468-468: LGTM: Korean phrasing is correct and concise.locales/en.yaml (1)
477-477: LGTM: English string is precise and matches behavior.locales/pt.yaml (1)
470-470: LGTM: Portuguese string conveys the restriction accurately.locales/it.yaml (1)
468-468: LGTM: Italian translation is correct and consistent with tone.locales/ru.yaml (1)
471-471: RU i18n: new key looks goodAccurate and consistent with the other locales’ intent.
locales/de.yaml (1)
471-471: DE i18n: new key is correctText reads naturally and matches intended meaning.
bot/modules/community/commands.ts (3)
20-26: Active-only search: good guardAdding active: true to discovery aligns with the new model.
75-75: Admin access over inactive communities: acknowledgedComment documents the intended exception clearly.
149-154: Allowing updates on inactive: OK, but verify intended surfaceGiven creators can reactivate communities, allowing updates on inactive ones is reasonable. Ensure admin UI includes a way to toggle active back on (not shown here).
bot/validations.ts (1)
112-121: Clearing inactive default community: good safeguardThis prevents stale defaults and aligns validations with active-only semantics.
bot/modules/user/scenes/settings.ts (1)
31-36: Settings: clear inactive default and display active groupSolid UX—prevents showing a dead default and requires no extra prompts.
jobs/index.ts (2)
8-8: Rename to disableCommunity wired correctly.Import switch from deleteCommunity → disableCommunity aligns with the new deactivation flow.
18-18: No staledeleteCommunityimports remain
Grep across the codebase found no import statements referencingdeleteCommunityin TypeScript files.bot/start.ts (1)
70-70: Updated import matches jobs/index change.disableCommunity is correctly imported from ../jobs.
bot/modules/community/actions.ts (1)
134-136: Withdraw on inactive communities — confirm product intent.You now block withdrawals when inactive. If creators must still withdraw accrued earnings post-deactivation, allow only when ctx.user._id === creator_id.
Do we want to permit creator-only withdrawals on inactive communities?
util/communityHelper.ts (1)
30-33: Active-only group resolution is correct.Exact-match regex plus active: true prevents routing through inactive groups.
| community = await Community.findOne({ group: regex, active: true }); | ||
| } else if (groupName[0] == '-') { | ||
| community = await Community.findOne({ group: groupName }); | ||
| community = await Community.findOne({ group: groupName, active: true }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Differentiate “not found” vs “inactive” on /setcomm
Right now inactive communities fall through as “not found”. Prefer explicit feedback using community_inactive.
- const regex = new RegExp(['^', groupName, '$'].join(''), 'i');
- community = await Community.findOne({ group: regex, active: true });
+ const regex = new RegExp(['^', groupName, '$'].join(''), 'i');
+ community = await Community.findOne({ group: regex });
+ if (community && !community.active) {
+ return await ctx.reply(ctx.i18n.t('community_inactive'));
+ }
} else if (groupName[0] == '-') {
- community = await Community.findOne({ group: groupName, active: true });
+ community = await Community.findOne({ group: groupName });
+ if (community && !community.active) {
+ return await ctx.reply(ctx.i18n.t('community_inactive'));
+ }
}
- if (!community) {
+ if (!community) {
return await ctx.reply(ctx.i18n.t('community_not_found'));
}Also applies to: 57-59
🤖 Prompt for AI Agents
In bot/modules/community/commands.ts around lines 53-56 (and similarly at
57-59), the current logic treats inactive communities as "not found" because it
only queries for active: true; change each lookup to first fetch the community
regardless of active status (remove active filter), then check community.exists:
if not found return community_not_found; else if community.active is false
return community_inactive; otherwise proceed as before. Apply this pattern to
both the regex lookup and the groupName ('-'-prefixed) lookup (and the
subsequent similar block at 57-59).
bot/validations.ts
Outdated
| const community = await Community.findOne({ _id: communityId, active: true }); | ||
| if (!community) return false; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ban check ignores inactive communities: good; fix Prettier pipeline
Logic is correct. CI reports Prettier mismatch (lines 737–746). Please run the formatter.
🏁 Script executed:
#!/bin/bash
# Format only the touched file and show diff
npm run format -- --write bot/validations.ts
git --no-pager diff -- bot/validations.tsLength of output: 114
Prettier formatting step failing—install Prettier and re-run formatter
CI logs show prettier: not found when running npm run format. Add Prettier to your devDependencies (e.g. npm install --save-dev prettier), then run
npx prettier --write bot/validations.tsto fix the formatting mismatch.
🤖 Prompt for AI Agents
In bot/validations.ts around lines 740-741 the CI is failing because Prettier is
not installed; add Prettier to devDependencies (e.g. run `npm install --save-dev
prettier` or `yarn add -D prettier`), optionally pin a version in package.json,
then run the formatter on the file (`npx prettier --write bot/validations.ts`)
and commit the formatted file so CI `npm run format` succeeds.
util/index.ts
Outdated
| const community = await Community.findOne({ _id: order.community_id, active: true }); | ||
| if (community === null) throw Error('Community was not found in DB'); | ||
| channel = community.dispute_channel; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t throw when inactive/missing; fall back to default dispute channel.
Throwing here can break dispute flows for existing orders after deactivation.
- const community = await Community.findOne({ _id: order.community_id, active: true });
- if (community === null) throw Error('Community was not found in DB');
- channel = community.dispute_channel;
+ const community = await Community.findOne({ _id: order.community_id, active: true });
+ if (community && community.dispute_channel) {
+ channel = community.dispute_channel;
+ }Run: npm run format
📝 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.
| const community = await Community.findOne({ _id: order.community_id, active: true }); | |
| if (community === null) throw Error('Community was not found in DB'); | |
| channel = community.dispute_channel; | |
| const community = await Community.findOne({ _id: order.community_id, active: true }); | |
| if (community && community.dispute_channel) { | |
| channel = community.dispute_channel; | |
| } |
🤖 Prompt for AI Agents
In util/index.ts around lines 345-347, the code currently throws if
Community.findOne returns null which breaks dispute flows for existing orders
after community deactivation; instead remove the throw, and if the community is
null or inactive assign channel to a safe default dispute channel (e.g., a
configured DEFAULT_DISPUTE_CHANNEL or a fallback from config/env) and optionally
log a warning; ensure channel is set to community.dispute_channel only when
community exists, and then run npm run format.
| const community = await Community.findOne({ _id: communityId, active: true }); | ||
| if (community === null) throw Error('Community was not found in DB'); | ||
| communityFee = communityFee * (community.fee / 100); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fee calc should treat inactive as “no community” instead of throwing.
Throwing propagates to order flows. Return the global-fee path when inactive/missing.
- const community = await Community.findOne({ _id: communityId, active: true });
- if (community === null) throw Error('Community was not found in DB');
- communityFee = communityFee * (community.fee / 100);
+ const community = await Community.findOne({ _id: communityId, active: true });
+ if (!community) {
+ return isGoldenHoneyBadger ? 0 : maxFee;
+ }
+ communityFee = communityFee * (community.fee / 100);Run: npm run format
📝 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.
| const community = await Community.findOne({ _id: communityId, active: true }); | |
| if (community === null) throw Error('Community was not found in DB'); | |
| communityFee = communityFee * (community.fee / 100); | |
| const community = await Community.findOne({ _id: communityId, active: true }); | |
| if (!community) { | |
| return isGoldenHoneyBadger ? 0 : maxFee; | |
| } | |
| communityFee = communityFee * (community.fee / 100); |
🤖 Prompt for AI Agents
In util/index.ts around lines 457 to 460, do not throw when Community.findOne
returns null or inactive; instead treat missing/inactive community as "no
community" and leave the previously computed global fee path intact. Change the
logic to fetch the community (as you already do), then only apply communityFee =
communityFee * (community.fee / 100) if a community was found (and active),
otherwise skip that multiplication so the global fee remains in effect. After
making the change, run: npm run format.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
bot/validations.ts (1)
740-743: Prettier still failing in CI—install and run formatterPrevious CI failure cites a Prettier mismatch around these lines and that Prettier isn’t installed. Add it to devDependencies and re-run the formatter.
#!/bin/bash npm i -D prettier npx prettier --write bot/validations.ts git diff -- bot/validations.ts
🧹 Nitpick comments (2)
bot/validations.ts (2)
113-121: Don’t drop creator/solver privileges when clearing inactive default; add audit logClearing
default_community_idmakescommunitynull, soisDisputeSolver(community, user)right after may turn false and unintentionally block the community creator from admin flows to reactivate. Please verify the reactivation path; if affected, pass the inactive community as a fallback just for role checks. Also add an info log for traceability.Minimal logging addition within this block:
// If default community exists but is inactive, clear it if (community && !community.active) { user.default_community_id = undefined; await user.save(); - community = null; + logger.info('Clearing inactive default community', { + userId: user._id, + communityId: community._id, + }); + community = null; }Outside this block (for role checking only), consider:
// declare before the block and set it before nulling `community` let inactiveCommunity: typeof community | null = null; // ... inactiveCommunity = community; // ... const isSolver = isDisputeSolver(community ?? inactiveCommunity, user);
740-743: Micro perf: avoid hydrating full documents hereWe only need
banned_users. Use projection +lean()to cut overhead.- const community = await Community.findOne({ - _id: communityId, - active: true, - }); + const community = await Community.findOne( + { _id: communityId, active: true }, + { banned_users: 1 } + ) + .lean() + .exec();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
bot/modules/community/actions.ts(3 hunks)bot/validations.ts(2 hunks)models/indexes.ts(1 hunks)util/index.ts(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- models/indexes.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- util/index.ts
- bot/modules/community/actions.ts
🧰 Additional context used
📓 Path-based instructions (2)
bot/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
bot/**/*.ts: Use custom Telegraf context types (MainContext, CommunityContext) in handlers and modules
Integrate @grammyjs/i18n for dynamic language selection based on user preferences
Files:
bot/validations.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use Winston for logging (avoid console) and include contextual details and timeout monitoring where relevant
Wrap risky operations in try-catch with meaningful error context
Files:
bot/validations.ts
🧬 Code graph analysis (1)
bot/validations.ts (1)
tests/bot/validation.spec.ts (7)
newCommunity(427-444)newCommunity(399-418)Community(1122-1126)Community(1128-1132)user(1134-1143)beforeEach(1104-1144)community(1105-1111)
🔇 Additional comments (1)
bot/validations.ts (1)
740-743: LGTM: ignore bans on inactive communitiesFiltering by
{ active: true }aligns with the new semantics; banned checks shouldn’t apply to inactive communities.
Change back after #697 is merge
* Fix test to pass after disable delete community Change back after #697 is merge * Apply prettier * Fix bot version on lock file
This commit introduces comprehensive validation to prevent operations on inactive communities while preserving administrative access for reactivation.
Key Changes:
Technical Implementation:
Impact:
Files Modified:
Summary by CodeRabbit
New Features
Bug Fixes
Chores