Skip to content
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

Issue 543 error displaying username with underscore #586

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions bot/modules/dispute/messages.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
const { getDisputeChannel, getDetailedOrder } = require('../../../util');
const { logger } = require('../../../logger');

const escapeMarkdown = (text) => text.replace(/_/g, '\\_');
Fixed Show fixed Hide fixed

exports.beginDispute = async (ctx, initiator, order, buyer, seller) => {
try {
let initiatorUser = buyer;
Expand Down Expand Up @@ -87,11 +89,16 @@ exports.disputeData = async (
}

const detailedOrder = getDetailedOrder(ctx.i18n, order, buyer, seller);

// Fix Issue 543: Escape underscores in usernames
const escapedInitiatorUsername = escapeMarkdown(initiatorUser.username);
const escapedCounterPartyUsername = escapeMarkdown(counterPartyUser.username);

await ctx.telegram.sendMessage(
solver.tg_id,
ctx.i18n.t('dispute_started_channel', {
initiatorUser,
counterPartyUser,
initiatorUser: { ...initiatorUser, username: escapedInitiatorUsername },
counterPartyUser: { ...counterPartyUser, username: escapedCounterPartyUsername },
buyer,
seller,
buyerDisputes,
Expand Down
4 changes: 2 additions & 2 deletions locales/en.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,8 @@ order_detail: |
seller: seller
buyer: buyer
dispute_started_channel: |
User ${type} @${initiatorUser.username}
has started a dispute with @${counterPartyUser.username} for the order
User ${type} @${initiatorUser.username} with the id: ${initiatorUser.tg_id}
has started a dispute with @${counterPartyUser.username} with the id: ${counterPartyUser.tg_id} for the order

${detailedOrder}

Expand Down
4 changes: 2 additions & 2 deletions locales/es.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ order_detail: |
seller: vendedor
buyer: comprador
dispute_started_channel: |
El ${type} @${initiatorUser.username}
ha iniciado una disputa con @${counterPartyUser.username} en la orden:
El ${type} @${initiatorUser.username} con la id: ${initiatorUser.tg_id}
ha iniciado una disputa con @${counterPartyUser.username} con la id: ${counterPartyUser.tg_id} en la orden:

${detailedOrder}

Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"lint": "eslint .",
"format": "prettier --write '**/*.js'",
"pretest": "npx tsc",
"test": "export NODE_ENV=test && mocha --exit tests/"
"test": "export NODE_ENV=test && mocha --exit 'tests/**/*.js'"
},
"license": "MIT",
"dependencies": {
Expand Down Expand Up @@ -53,6 +53,7 @@
"mocha": "^9.2.2",
"nodemon": "2.0.19",
"prettier": "^2.6.2",
"proxyquire": "^2.1.3",
"sinon": "^11.1.2",
"telegram-test-api": "^2.5.0",
"typescript": "^5.1.6"
Expand Down
108 changes: 108 additions & 0 deletions tests/bot/modules/dispute/messages.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
const { expect } = require('chai');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danfercf1 hey Daniel, as I told you in another PR, new files should go in TypeScript, not JavaScript

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it's a test file, and it will not be executed by the script of package.json if it's a typescript file

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test files can be written in typescript too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but they are not supported by the project

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make it so?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree, but itsn't part of this task

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, given no .ts tests exists in master branch, you cannot technically create a PR that separates that task completely. I mean, that task is needed only now that you're creating a new test

Copy link
Contributor Author

@danfercf1 danfercf1 Sep 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean that is necessary to prepare the configuration and dependencies to achieve that, it's not only an addition of a .ts file, all the stuff necessary could be a new PR and task

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey guys, I also agree new files should be .ts, but if we need to some some extra stuff to make it work I consider we should merge this as javascript and then work on migrating it to ts in another issue

const sinon = require('sinon');
const proxyquire = require('proxyquire');

const getDetailedOrderStub = sinon.stub().returns('Mocked order detail');
const { disputeData } = proxyquire('../../../../bot/modules/dispute/messages', {
'../../../util': { getDetailedOrder: getDetailedOrderStub }
});

// Mock dependencies
const mockTelegram = {
sendMessage: sinon.stub(),
};

const mockI18n = {
t: sinon.stub((key, params) => {
switch (key) {
case 'dispute_started_channel':
return `Dispute started by ${params.initiatorUser.username} against ${params.counterPartyUser.username} with tokens ${params.sellerToken} and ${params.buyerToken}`;
case 'seller':
return 'seller';
case 'buyer':
return 'buyer';
case 'dispute_solver':
return `Dispute taken by ${params.solver} with token ${params.token}`;
default:
return '';
}
}),
};

const mockCtx = {
telegram: mockTelegram,
i18n: mockI18n,
};

const mockOrder = {
seller_dispute_token: 'seller_token',
buyer_dispute_token: 'buyer_token',
};

const mockBuyerUnderscore = { username: 'buyer_user', tg_id: '246802' };
const mockBuyerNormal = { username: 'buyer-user', tg_id: '246802' };
const mockSeller = { username: 'seller_user', tg_id: '567890' };
const mockSolver = { username: 'solver', tg_id: '123456' };

describe('disputeData', () => {
afterEach(() => {
sinon.restore();
});

it('should send a message with escaped underscores in usernames', async () => {
await disputeData(
mockCtx,
mockBuyerUnderscore,
mockSeller,
mockOrder,
'buyer',
mockSolver,
5,
3
);

expect(mockTelegram.sendMessage.calledWith(
mockSolver.tg_id,
sinon.match(/buyer\\_user/),
sinon.match({ parse_mode: 'MarkdownV2' })
)).to.be.true;
});

it('should send a message without underscores in usernames', async () => {
await disputeData(
mockCtx,
mockBuyerNormal,
mockSeller,
mockOrder,
'buyer',
mockSolver,
5,
3
);

expect(mockTelegram.sendMessage.calledWith(
mockSolver.tg_id,
sinon.match('buyer-user'),
sinon.match({ parse_mode: 'MarkdownV2' })
)).to.be.true;
});

it('should swap initiator and counterparty if initiator is seller', async () => {
await disputeData(
mockCtx,
mockBuyerNormal,
mockSeller,
mockOrder,
'seller',
mockSolver,
5,
3
);

expect(mockTelegram.sendMessage.calledWith(
mockSolver.tg_id,
sinon.match(/seller\\_user/),
sinon.match({ parse_mode: 'MarkdownV2' })
)).to.be.true;
});
});