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.

Sign up for GitHub

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

Jump to bottom

bal 1589 rules after schema changes #2187

Merged
merged 13 commits into from
Mar 12, 2024
Merged

bal 1589 rules after schema changes #2187

merged 13 commits into from
Mar 12, 2024

Conversation

liorzam
Copy link
Collaborator

@liorzam liorzam commented Mar 11, 2024 •
edited
Loading

No description provided.

@liorzblrn
Squashed commit of the following:
04307e0 
commit 13e2390bf0940df4e3276b64c11b2c75cb7400eb
Author: Lior Zamir <[email protected]>
Date:   Mon Mar 11 16:21:33 2024 +0200

    chore: remove non used rules

commit 34f8eff
Author: Lior Zamir <[email protected]>
Date:   Mon Mar 11 16:17:20 2024 +0200

    fix: add missing private functions

commit e5d7552
Merge: 51afe72 00aa41e
Author: Lior Zamir <[email protected]>
Date:   Wed Mar 6 01:22:29 2024 +0200

    chore: merge branch

commit 51afe72
Author: Lior Zamir <[email protected]>
Date:   Wed Mar 6 01:19:39 2024 +0200

    fix: ts issues

commit fe9cf42
Author: Lior Zamir <[email protected]>
Date:   Wed Mar 6 01:15:56 2024 +0200

    fix: ts issues

commit 00aa41e
Author: Lior Zamir <[email protected]>
Date:   Wed Mar 6 00:59:53 2024 +0200

    fix(generate-alerts.ts): import missing types and add missing type annotation for TransactionRecordType
    fix(alert.service.ts): add error handling when checking alert and log error message and definition ID

    fix(data-analytics.service.ts): fix formatting and indentation for better readability
    feat(data-analytics.service.ts): add support for evaluating payment unexpected based on customer expected amount
    feat(data-analytics.service.ts): add support for evaluating dormant accounts based on transaction history
    feat(data-analytics.service.ts): add support for evaluating customers' transaction type based on various criteria

    feat(types.ts): add support for new transaction record type and generic async function type
    feat(types.ts): add new options for TCustomersTransactionTypeOptions to support more flexible customer transaction filtering
    feat(types.ts): add new options for TDormantAccountOption to support evaluating dormant accounts
    feat(types.ts): add new evaluate functions for evaluating transactions against dynamic rules and dormant accounts

commit 7b0dddf
Author: Lior Zamir <[email protected]>
Date:   Wed Mar 6 00:59:53 2024 +0200

    feat: add more rules
@liorzblrn
fix: rule logic changes 1
c36540a
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Mar 11, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: 53fe5d7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@liorzam liorzam changed the base branch from dev to epic/bal-1521-tm March 11, 2024 15:51
MatanYadaev
MatanYadaev approved these changes Mar 12, 2024
View reviewed changes
services/workflows-service/src/prisma/prisma.service.ts Show resolved Hide resolved
services/workflows-service/scripts/alerts/generate-alerts.ts
@@ -91,7 +104,7 @@ export const ALERT_INLINE_RULES = [
havingAggregate: AggregateType.COUNT,

direction: 'inbound',
excludedCounterpartyIds: ['9999999999999999', '999999******9999'],
// excludedCounterparty: ['9999999999999999', '999999******9999'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider deleting this line instead of commenting it.

services/workflows-service/scripts/alerts/generate-alerts.ts
@@ -119,7 +132,7 @@ export const ALERT_INLINE_RULES = [
havingAggregate: AggregateType.COUNT,

direction: 'inbound',
excludedCounterpartyIds: ['9999999999999999', '999999******9999'],
// excludedCounterparty: ['9999999999999999', '999999******9999'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we need to modify this exclusion from other rules.
so once i changed the type i had to take care for it cross all other rules

services/workflows-service/src/alert/alert.service.ts Outdated
Comment on lines 21 to 22
for (let p in obj) {
if (p in obj && prop === `${p}`.toLowerCase()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use .hasOwnProperty

Suggested change
for (let p in obj) {
if (p in obj && prop === `${p}`.toLowerCase()) {
for (const p in obj) {
if (obj.hasOwnProperty(p) && prop === p.toLowerCase()) {

Or use Object.keys()

Suggested change
for (let p in obj) {
if (p in obj && prop === `${p}`.toLowerCase()) {
for (const p of Object.keys(obj)) {
if (prop === p.toLowerCase()) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

property can be also non string
that's why i've used ${p}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

eslint rule
7:13 error Do not access Object.prototype method 'hasOwnProperty' from target object no-prototype-builtins

services/workflows-service/src/alert/alert.service.ts Outdated Show resolved Hide resolved
services/workflows-service/src/alert/alert.service.ts Outdated Show resolved Hide resolved
services/workflows-service/src/alert/alert.service.ts Outdated Show resolved Hide resolved
@liorzam liorzam self-assigned this Mar 12, 2024
@liorzam liorzam marked this pull request as ready for review March 12, 2024 20:35
@liorzam liorzam merged commit 8a70760 into epic/bal-1521-tm Mar 12, 2024
7 checks passed
@liorzam liorzam deleted the bal-1589-rules-after-schema-changes branch March 12, 2024 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MatanYadaev MatanYadaev MatanYadaev approved these changes

Assignees

@liorzam liorzam

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@liorzam @MatanYadaev @liorzblrn