refactor(condo): DOMA-10610 remove moneyToWords and renderMoney #5688

tolmachev21 · 2025-01-13T14:08:29Z

Small changes, which removed suppord currecy display for different locales. And add lib, which conversation number to words.

How now looks document, u can see here:

SavelevMatthew · 2025-01-13T14:11:33Z

apps/condo/domains/ticket/utils/serverSchema/TicketDocumentGeneration/completionWorks.js

+                    style: 'currency',
+                    currency: currencyCode,
+                    currencyDisplay: 'code',
+                }).format(price * row.count).replace(/[A-Z]{3}/, '').trim() : '',


'' is bad default value to store...

This code is executed on the server because this task. An empty string is the value that will be inserted into the document (for us, an empty string is an empty space in the document that can be filled in manually). This value is not stored in the DB

SavelevMatthew

I'm a bit missing this PR.

Is this code used on server or client?

If on client, why its in serverSchema
And if on server, why do you default values ''. Empty string is not good value to store in DB, consider null / undefined or throw an exception.

Why do you need this strange import? Especially globally defined...Also seems strange to me. Please explain, what exactly is happening

tolmachev21 · 2025-01-13T14:42:08Z

Is this code used on server or client?

If on client, why its in serverSchema And if on server, why do you default values ''. Empty string is not good value to store in DB, consider null / undefined or throw an exception.

Is this code used on server or client? - An empty string is a value that will be inserted into the document. It is not saved in the database.

Why do you need this strange import? Especially globally defined...Also seems strange to me. Please explain, what exactly is happening

Why do you need this strange import? - This import is used here because the n2words library does not support СommunJS modules. Since we are currently on 16 node version, we also do not have support for ESM modules.

The solution to this situation is the dynamic import function - import(), which is supported since 12 version node and makes it possible to use ESM modules in files with CommunJS import

Alllex202 · 2025-01-15T13:37:03Z

apps/condo/domains/ticket/utils/serverSchema/TicketDocumentGeneration/completionWorks.js

+                    style: 'currency',
+                    currency: currencyCode,
+                    currencyDisplay: 'code',
+                }).format(price).replace(/[A-Z]{3}/, '').trim() : '',


Please move the regular expression to a constant with a clear name

Alllex202 · 2025-01-15T13:39:53Z

apps/condo/domains/ticket/utils/serverSchema/TicketDocumentGeneration/completionWorks.js

+                price: !Number.isNaN(parseFloat(price)) ? new Intl.NumberFormat(locale, {
+                    style: 'currency',
+                    currency: currencyCode,
+                    currencyDisplay: 'code',
+                }).format(price).replace(/[A-Z]{3}/, '').trim() : '',


Why are you using parseFloat(price) again? You've already called this function before

Alllex202 · 2025-01-15T13:42:54Z

apps/condo/domains/ticket/utils/serverSchema/TicketDocumentGeneration/completionWorks.js

+                    currency: currencyCode,
+                    currencyDisplay: 'code',
+                }).format(price).replace(/[A-Z]{3}/, '').trim() : '',
+                sum: !Number.isNaN(parseFloat(price * row.count)) ? new Intl.NumberFormat(locale, {


parseFloat ^^

Alllex202 · 2025-01-15T14:11:08Z

apps/condo/domains/ticket/utils/serverSchema/TicketDocumentGeneration/paidWorks.js

+            wholePartTotalSum: n2words(numberFormatByLocale
+                .formatToParts(totalSum)
+                .map(p => p.type === 'integer' ? p.value : '')
+                .join(''), { lang: locale }),
+            decimalPartTotalSum:  n2words(numberFormatByLocale
+                .formatToParts(totalSum)
+                .map(p => p.type === 'fraction' ? p.value : '')
+                .join(''), { lang: locale, feminine: true }),
+            wholePartTotalVAT: n2words(numberFormatByLocale
+                .formatToParts(totalVAT)
+                .map(p => p.type === 'integer' ? p.value : '')
+                .join(''), { lang: locale }),
+            decimalPartTotalVAT: n2words(numberFormatByLocale
+                .formatToParts(totalVAT)
+                .map(p => p.type === 'fraction' ? p.value : '')
+                .join(''), { lang: locale, feminine: true }),


It seems that the need to use the feminine parameter depends on the locale.

Right now it is not very important (since our document is only in Russian), but it is worth keeping in mind

add a if ru -> use feminine

toplenboren · 2025-01-15T15:34:32Z

apps/condo/domains/ticket/utils/serverSchema/TicketDocumentGeneration/paidWorks.js

 const { TICKET_DOCUMENT_GENERATION_TASK_FORMAT } = require('@condo/domains/ticket/constants/ticketDocument')

 const logger = getLogger('generateDocumentOfPaidWorksCompletion')

+const n2wordsImport = import('n2words').then(module => module.default)


add a comment describing why this syntax is used

toplenboren · 2025-01-15T15:36:16Z

apps/condo/domains/ticket/utils/serverSchema/TicketDocumentGeneration/paidWorks.js

+                .formatToParts(totalSum)
+                .map(p => p.type === 'fraction' ? p.value : '')
+                .join(''), { lang: locale, feminine: true }),
+            wholePartTotalVAT: n2words(numberFormatByLocale


naming-wise its best to specify entity first and then specify the type

TotalWholePart
TotalDecimalPart

VATWholePart
VATDecimalPart

toplenboren · 2025-01-15T15:36:33Z

apps/condo/domains/ticket/utils/serverSchema/TicketDocumentGeneration/paidWorks.js

+            wholePartTotalSum: n2words(numberFormatByLocale
+                .formatToParts(totalSum)
+                .map(p => p.type === 'integer' ? p.value : '')
+                .join(''), { lang: locale }),
+            decimalPartTotalSum:  n2words(numberFormatByLocale
+                .formatToParts(totalSum)
+                .map(p => p.type === 'fraction' ? p.value : '')
+                .join(''), { lang: locale, feminine: true }),
+            wholePartTotalVAT: n2words(numberFormatByLocale
+                .formatToParts(totalVAT)
+                .map(p => p.type === 'integer' ? p.value : '')
+                .join(''), { lang: locale }),
+            decimalPartTotalVAT: n2words(numberFormatByLocale
+                .formatToParts(totalVAT)
+                .map(p => p.type === 'fraction' ? p.value : '')
+                .join(''), { lang: locale, feminine: true }),


add a if ru -> use feminine

sonarqubecloud · 2025-01-31T17:30:30Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

tolmachev21 temporarily deployed to internal January 13, 2025 14:08 — with GitHub Actions Inactive

SavelevMatthew reviewed Jan 13, 2025

View reviewed changes

SavelevMatthew requested changes Jan 13, 2025

View reviewed changes

tolmachev21 mentioned this pull request Jan 13, 2025

feat(condo): DOMA-10397 add act of paid works in ticket #5370

Merged

tolmachev21 force-pushed the refactor/condo/DOMA-10610/remove-support-different-currencies branch from 42ce141 to 352bdaa Compare January 14, 2025 08:01

tolmachev21 temporarily deployed to internal January 14, 2025 08:01 — with GitHub Actions Inactive

Alllex202 reviewed Jan 15, 2025

View reviewed changes

toplenboren approved these changes Jan 15, 2025

View reviewed changes

tolmachev21 force-pushed the refactor/condo/DOMA-10610/remove-support-different-currencies branch from 352bdaa to 4721fa6 Compare January 17, 2025 13:23

tolmachev21 temporarily deployed to internal January 17, 2025 13:23 — with GitHub Actions Inactive

tolmachev21 temporarily deployed to internal January 17, 2025 14:25 — with GitHub Actions Inactive

Alllex202 approved these changes Jan 17, 2025

View reviewed changes

tolmachev21 added 5 commits January 31, 2025 22:24

refactor(condo): DOMA-10610 remove moneyToWords and renderMoney

4b5a94b

refactor(condo): DOMA-10610 changed getCondition on getAll

ffa0968

refactor(condo): DOMA-10610 fix review

e77a70e

refactor(condo): DOMA-10610 change file

dd8b30d

fix(condo): DOMA-10610 update jest config

8c18fba

tolmachev21 force-pushed the refactor/condo/DOMA-10610/remove-support-different-currencies branch from 3f417fb to 8c18fba Compare January 31, 2025 17:25

tolmachev21 temporarily deployed to internal January 31, 2025 17:25 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(condo): DOMA-10610 remove moneyToWords and renderMoney #5688

refactor(condo): DOMA-10610 remove moneyToWords and renderMoney #5688

tolmachev21 commented Jan 13, 2025

SavelevMatthew Jan 13, 2025

tolmachev21 Jan 13, 2025

SavelevMatthew left a comment

tolmachev21 commented Jan 13, 2025

Alllex202 Jan 15, 2025

Alllex202 Jan 15, 2025

Alllex202 Jan 15, 2025

Alllex202 Jan 15, 2025

toplenboren Jan 15, 2025

toplenboren Jan 15, 2025

toplenboren Jan 15, 2025

toplenboren Jan 15, 2025

sonarqubecloud bot commented Jan 31, 2025

refactor(condo): DOMA-10610 remove moneyToWords and renderMoney #5688

Are you sure you want to change the base?

refactor(condo): DOMA-10610 remove moneyToWords and renderMoney #5688

Conversation

tolmachev21 commented Jan 13, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SavelevMatthew left a comment

Choose a reason for hiding this comment

tolmachev21 commented Jan 13, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 31, 2025

Quality Gate passed