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

Feat(condo): DOMA-10067 normalizing unit name #5191

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

YEgorLu
Copy link
Contributor

@YEgorLu YEgorLu commented Sep 5, 2024

added function for unit name normalization and used it in allResidentBillingReceipts.

Problem in allResidentBillingReceipts was that user can have unit name like '12a', and billing receipt says '12A' and we needed way to say that they are equal

There are many cases when users describe unitName strangely, tests may not have covered all of them, so the plan is to extend normalization in the future if needed

Added slugify package to make translation of symbols from other languages to english more convenient.

…cessary test cases, fixed roman numbers with ru locale, removing emojis
…s when user and billingReceipt have same unitName but with words in different registries
@YEgorLu YEgorLu added ✋🙂 Review please Comments are resolved, take a look, please 😵‍💫 dependencies Pull requests that update a dependency file / yarn.lock labels Sep 5, 2024
Copy link

sonarcloud bot commented Sep 5, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots

See analysis details on SonarCloud

Copy link
Member

Choose a reason for hiding this comment

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

It's common practice to use functions instead of anonymous arrow function when making utils

Copy link
Member

Choose a reason for hiding this comment

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

it's easier to debug and understan the traces

['0-1', '0-1'],
['3-8-13\n', '3-8-13'],
['050-2/1', '050-2-1'],
['кл.кл. 3 (19Б кв. 14)', '3-19-b-14'],
Copy link
Member

Choose a reason for hiding this comment

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

where is kl?

Copy link
Member

Choose a reason for hiding this comment

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

Who is responsible for list of abbreviations?

Copy link
Member

@SavelevMatthew SavelevMatthew left a comment

Choose a reason for hiding this comment

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

This feature needs to be discussed first, I've got a lot of questions related to the decisions you've and @dkoviazin made

@dkoviazin dkoviazin added the 🔬 WIP Not intended to be merged right now, it is a work in progress label Sep 5, 2024
@SavelevMatthew SavelevMatthew marked this pull request as draft September 5, 2024 16:45
@YEgorLu YEgorLu removed the ✋🙂 Review please Comments are resolved, take a look, please label Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😵‍💫 dependencies Pull requests that update a dependency file / yarn.lock 🔬 WIP Not intended to be merged right now, it is a work in progress
Development

Successfully merging this pull request may close these issues.

4 participants