Skip to content

Commit ac1413d

Browse files
committed
Fix review findings on counterparty requirements (PR #853)
- Parse command arguments with split(/\s+/): with split(' ') a double space made value bind to '' and Number('') === 0, silently storing 0 and disabling the requirement the user was trying to set. - Split not_meeting_requirements into not_meeting_age_requirement and not_meeting_orders_requirement (all 10 locales) so a rejected taker is only told the threshold they actually failed, instead of a message citing an unset requirement as "0". - Re-render the settings message in resetMessage before handling a command, so repeating a command with the same value still produces a visible acknowledgement instead of being skipped by the messageChanged guard. - Share the non-negative-integer predicate between readNonNegativeInt and the command argument validation instead of duplicating it. - /resetrequirements now unsets counterparty_requirements instead of storing zeros, keeping a single canonical representation of "no requirements" that matches the gate's fast path. - Update takeOrder.spec.ts for the per-threshold denial messages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KLpk56FhD2oPtF7ne7UxT9
1 parent e3e5bad commit ac1413d

14 files changed

Lines changed: 107 additions & 53 deletions

File tree

bot/messages.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -999,15 +999,30 @@ const userTakerIsBlockedByUserOrder = async (
999999
}
10001000
};
10011001

1002-
const notMeetingRequirementsMessage = async (
1002+
const notMeetingAgeRequirementMessage = async (
10031003
ctx: MainContext,
10041004
user: UserDocument,
1005-
requirements?: { min_days_using_bot: number; min_completed_orders: number },
1005+
min_days_using_bot: number,
10061006
) => {
10071007
try {
10081008
await ctx.telegram.sendMessage(
10091009
user.tg_id,
1010-
ctx.i18n.t('not_meeting_requirements', requirements),
1010+
ctx.i18n.t('not_meeting_age_requirement', { min_days_using_bot }),
1011+
);
1012+
} catch (error) {
1013+
logger.error(error);
1014+
}
1015+
};
1016+
1017+
const notMeetingOrdersRequirementMessage = async (
1018+
ctx: MainContext,
1019+
user: UserDocument,
1020+
min_completed_orders: number,
1021+
) => {
1022+
try {
1023+
await ctx.telegram.sendMessage(
1024+
user.tg_id,
1025+
ctx.i18n.t('not_meeting_orders_requirement', { min_completed_orders }),
10111026
);
10121027
} catch (error) {
10131028
logger.error(error);
@@ -2263,5 +2278,6 @@ export {
22632278
userTakerIsBlockedByUserOrder,
22642279
userOrderIsBlockedByUserTaker,
22652280
showQRCodeMessage,
2266-
notMeetingRequirementsMessage,
2281+
notMeetingAgeRequirementMessage,
2282+
notMeetingOrdersRequirementMessage,
22672283
};

bot/modules/orders/takeOrder.ts

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -174,22 +174,25 @@ export const meetsCounterpartyRequirements = async (
174174
// predate created_at tracking and are genuinely old, so we let them pass
175175
// the age check explicitly instead of relying on NaN comparisons.
176176
if (!Number.isNaN(ageInDays) && ageInDays < min_days_using_bot) {
177-
await messages.notMeetingRequirementsMessage(ctx, user, {
177+
await messages.notMeetingAgeRequirementMessage(
178+
ctx,
179+
user,
178180
min_days_using_bot,
179-
min_completed_orders,
180-
});
181+
);
181182
return false;
182183
}
183184
}
184185

185-
if (min_completed_orders > 0) {
186-
if (user.trades_completed < min_completed_orders) {
187-
await messages.notMeetingRequirementsMessage(ctx, user, {
188-
min_days_using_bot,
189-
min_completed_orders,
190-
});
191-
return false;
192-
}
186+
if (
187+
min_completed_orders > 0 &&
188+
user.trades_completed < min_completed_orders
189+
) {
190+
await messages.notMeetingOrdersRequirementMessage(
191+
ctx,
192+
user,
193+
min_completed_orders,
194+
);
195+
return false;
193196
}
194197

195198
return true;

bot/modules/user/scenes/settings.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@ import {
99
import { Message } from 'telegraf/typings/core/types/typegram';
1010
import { logger } from '../../../../logger';
1111

12+
const isNonNegativeInt = (n: number) => Number.isInteger(n) && n >= 0;
13+
1214
const readNonNegativeInt = (value: string | undefined, fallback: number) => {
1315
if (value === undefined || value.trim() === '') return fallback;
1416
const parsed = Number(value);
15-
return Number.isInteger(parsed) && parsed >= 0 ? parsed : fallback;
17+
return isNonNegativeInt(parsed) ? parsed : fallback;
1618
};
1719

1820
const DEFAULT_COUNTERPARTY_REQUIREMENTS = {
@@ -28,8 +30,15 @@ const DEFAULT_MAX_COUNTERPARTY_ORDERS = 10;
2830
function make() {
2931
const resetMessage = async (ctx: CommunityContext, next: () => void) => {
3032
const state = ctx.scene.state as CommunityWizardState;
31-
delete state.feedback;
32-
delete state.error;
33+
// Re-render without the previous feedback/error line so the next
34+
// updateMessage always differs from the displayed text; otherwise
35+
// repeating a command with the same value would be silently skipped
36+
// by the messageChanged guard, leaving the user with no acknowledgement.
37+
if (state.feedback || state.error) {
38+
delete state.feedback;
39+
delete state.error;
40+
await updateMessage(ctx);
41+
}
3342
next();
3443
};
3544
async function mainData(ctx: CommunityContext) {
@@ -176,10 +185,11 @@ function make() {
176185
const state = ctx.scene.state as CommunityWizardState;
177186
if (ctx.message === undefined || !('text' in ctx.message))
178187
throw new Error('ctx.message is undefined');
179-
const [, value] = ctx.message.text.trim().split(' ');
188+
// Split on runs of whitespace: with split(' ') a double space would
189+
// bind value to '' and Number('') === 0 silently disables the rule.
190+
const [, value] = ctx.message.text.trim().split(/\s+/);
180191
const parsed = Number(value);
181-
if (!Number.isInteger(parsed) || parsed < 0)
182-
throw new Error('NotValidNumber');
192+
if (!isNonNegativeInt(parsed)) throw new Error('NotValidNumber');
183193
const max = readNonNegativeInt(process.env[envVar], fallbackMax);
184194
if (parsed > max) {
185195
state.error = {
@@ -238,9 +248,10 @@ function make() {
238248
await ctx.deleteMessage();
239249
const state = ctx.scene.state as CommunityWizardState;
240250
const user = state.user;
241-
user.counterparty_requirements = {
242-
...DEFAULT_COUNTERPARTY_REQUIREMENTS,
243-
};
251+
// Unset the field instead of storing zeros so "no requirements"
252+
// keeps a single canonical representation (undefined), matching the
253+
// fast path in meetsCounterpartyRequirements.
254+
user.counterparty_requirements = undefined;
244255
await user.save();
245256
state.feedback = { i18n: 'requirements_reset' };
246257
await updateMessage(ctx);

locales/de.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -656,7 +656,8 @@ user_settings: |
656656
/counterpartyorders &lt;Aufträge&gt; - Legt die Mindestanzahl abgeschlossener Aufträge fest.
657657
/resetrequirements - Setzt die Gegenpartei-Anforderungen auf Standardwerte zurück.
658658
/exit - um den Assistenten zu verlassen.
659-
not_meeting_requirements: Du erfüllst nicht die Anforderungen der Gegenpartei, um diesen Auftrag anzunehmen (erfordert mindestens ${min_days_using_bot} Tage Nutzung des Bots und ${min_completed_orders} abgeschlossene Aufträge).
659+
not_meeting_age_requirement: Du erfüllst nicht die Anforderungen der Gegenpartei, um diesen Auftrag anzunehmen (erfordert mindestens ${min_days_using_bot} Tage Nutzung des Bots).
660+
not_meeting_orders_requirement: Du erfüllst nicht die Anforderungen der Gegenpartei, um diesen Auftrag anzunehmen (erfordert mindestens ${min_completed_orders} abgeschlossene Aufträge).
660661
counterpartyage_updated: Altersanforderung der Gegenpartei auf ${days} Tage aktualisiert.
661662
counterpartyorders_updated: Auftragsanforderung der Gegenpartei auf ${orders} aktualisiert.
662663
requirements_reset: Gegenpartei-Anforderungen wurden auf Standardwerte zurückgesetzt.

locales/en.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -663,7 +663,8 @@ user_settings: |
663663
/counterpartyorders &lt;orders&gt; - Sets the minimum number of completed orders for the counterparty.
664664
/resetrequirements - Resets counterparty requirements to default.
665665
/exit - to exit the wizard.
666-
not_meeting_requirements: You do not meet the counterparty requirements to take this order (requires at least ${min_days_using_bot} days using the bot and ${min_completed_orders} completed orders).
666+
not_meeting_age_requirement: You do not meet the counterparty requirements to take this order (requires at least ${min_days_using_bot} days using the bot).
667+
not_meeting_orders_requirement: You do not meet the counterparty requirements to take this order (requires at least ${min_completed_orders} completed orders).
667668
counterpartyage_updated: Counterparty age requirement updated to ${days} days.
668669
counterpartyorders_updated: Counterparty completed orders requirement updated to ${orders}.
669670
requirements_reset: Counterparty requirements have been reset to default.

locales/es.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,8 @@ user_settings: |
659659
/counterpartyorders &lt;órdenes&gt; - Establece el mínimo de órdenes completadas para la contraparte.
660660
/resetrequirements - Restablece los requisitos de contraparte al valor por defecto.
661661
/exit - para salir del asistente.
662-
not_meeting_requirements: No cumples los requisitos de la contraparte para tomar esta orden (requiere al menos ${min_days_using_bot} días usando el bot y ${min_completed_orders} órdenes completadas).
662+
not_meeting_age_requirement: No cumples los requisitos de la contraparte para tomar esta orden (requiere al menos ${min_days_using_bot} días usando el bot).
663+
not_meeting_orders_requirement: No cumples los requisitos de la contraparte para tomar esta orden (requiere al menos ${min_completed_orders} órdenes completadas).
663664
counterpartyage_updated: Requisito de antigüedad de contraparte actualizado a ${days} días.
664665
counterpartyorders_updated: Requisito de órdenes completadas de contraparte actualizado a ${orders}.
665666
requirements_reset: Los requisitos de contraparte han sido restablecidos al valor por defecto.

locales/fa.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -757,7 +757,8 @@ user_settings: |
757757
758758
خروج از راهنما:
759759
/exit
760-
not_meeting_requirements: شما واجد الزامات طرف مقابل برای پذیرش این سفارش نیستید (حداقل ${min_days_using_bot} روز استفاده از ربات و ${min_completed_orders} سفارش تکمیل‌شده لازم است).
760+
not_meeting_age_requirement: شما واجد الزامات طرف مقابل برای پذیرش این سفارش نیستید (حداقل ${min_days_using_bot} روز استفاده از ربات لازم است).
761+
not_meeting_orders_requirement: شما واجد الزامات طرف مقابل برای پذیرش این سفارش نیستید (حداقل ${min_completed_orders} سفارش تکمیل‌شده لازم است).
761762
counterpartyage_updated: الزامات سابقه طرف مقابل به ${days} روز به‌روزرسانی شد.
762763
counterpartyorders_updated: الزامات سفارش‌های تکمیل شده طرف مقابل به ${orders} عدد به‌روزرسانی شد.
763764
requirements_reset: الزامات طرف مقابل به مقادیر پیش‌فرض بازگردانده شد.

locales/fr.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -655,7 +655,8 @@ user_settings: |
655655
/counterpartyorders &lt;ordres&gt; - Définit le nombre minimum d'ordres complétés pour la contrepartie.
656656
/resetrequirements - Réinitialise les exigences de contrepartie aux valeurs par défaut.
657657
/exit - pour quitter l'assistant.
658-
not_meeting_requirements: Vous ne remplissez pas les exigences de la contrepartie pour prendre cet ordre (nécessite au moins ${min_days_using_bot} jours d'utilisation du bot et ${min_completed_orders} ordres complétés).
658+
not_meeting_age_requirement: Vous ne remplissez pas les exigences de la contrepartie pour prendre cet ordre (nécessite au moins ${min_days_using_bot} jours d'utilisation du bot).
659+
not_meeting_orders_requirement: Vous ne remplissez pas les exigences de la contrepartie pour prendre cet ordre (nécessite au moins ${min_completed_orders} ordres complétés).
659660
counterpartyage_updated: Exigence d'âge de la contrepartie mise à jour à ${days} jours.
660661
counterpartyorders_updated: Exigence d'ordres complétés de la contrepartie mise à jour à ${orders}.
661662
requirements_reset: Les exigences de contrepartie ont été réinitialisées aux valeurs par défaut.

locales/it.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -653,7 +653,8 @@ user_settings: |
653653
/counterpartyorders &lt;ordini&gt; - Imposta il numero minimo di ordini completati per la controparte.
654654
/resetrequirements - Ripristina i requisiti della controparte ai valori predefiniti.
655655
/exit - per uscire dall'assistente.
656-
not_meeting_requirements: Non soddisfi i requisiti della controparte per accettare questo ordine (sono necessari almeno ${min_days_using_bot} giorni di utilizzo del bot e ${min_completed_orders} ordini completati).
656+
not_meeting_age_requirement: Non soddisfi i requisiti della controparte per accettare questo ordine (sono necessari almeno ${min_days_using_bot} giorni di utilizzo del bot).
657+
not_meeting_orders_requirement: Non soddisfi i requisiti della controparte per accettare questo ordine (sono necessari almeno ${min_completed_orders} ordini completati).
657658
counterpartyage_updated: Requisito di età della controparte aggiornato a ${days} giorni.
658659
counterpartyorders_updated: Requisito di ordini completati della controparte aggiornato a ${orders}.
659660
requirements_reset: I requisiti della controparte sono stati ripristinati ai valori predefiniti.

locales/ko.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -653,7 +653,8 @@ user_settings: |
653653
/counterpartyorders &lt;주문&gt; - 거래 상대방의 최소 완료 주문 수를 설정합니다.
654654
/resetrequirements - 거래 상대방 요구 사항을 기본값으로 초기화합니다.
655655
/exit - 설정 모드 나가기.
656-
not_meeting_requirements: 귀하는 이 주문을 수락하기 위한 거래 상대방 요구 사항을 충족하지 않습니다 (최소 ${min_days_using_bot}일의 봇 사용과 ${min_completed_orders}개의 완료된 주문이 필요합니다).
656+
not_meeting_age_requirement: 귀하는 이 주문을 수락하기 위한 거래 상대방 요구 사항을 충족하지 않습니다 (최소 ${min_days_using_bot}일의 봇 사용이 필요합니다).
657+
not_meeting_orders_requirement: 귀하는 이 주문을 수락하기 위한 거래 상대방 요구 사항을 충족하지 않습니다 (최소 ${min_completed_orders}개의 완료된 주문이 필요합니다).
657658
counterpartyage_updated: 거래 상대방 가입 기간 요구 사항이 ${days}일로 업데이트되었습니다.
658659
counterpartyorders_updated: 거래 상대방 완료 주문 수 요구 사항이 ${orders}개로 업데이트되었습니다.
659660
requirements_reset: 거래 상대방 요구 사항이 기본값으로 초기화되었습니다.

0 commit comments

Comments
 (0)