Skip to content

send interest email#1179

Open
cristianizzo wants to merge 4 commits into
developfrom
AMNCARD-119
Open

send interest email#1179
cristianizzo wants to merge 4 commits into
developfrom
AMNCARD-119

Conversation

@cristianizzo
Copy link
Copy Markdown
Member

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 27, 2021

Codecov Report

Merging #1179 (6274354) into develop (48ab175) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 6274354 differs from pull request most recent head 8411c52. Consider uploading reports for the commit 8411c52 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1179      +/-   ##
===========================================
- Coverage    99.65%   99.65%   -0.01%     
===========================================
  Files          318      317       -1     
  Lines        10758    10659      -99     
  Branches      1562     1541      -21     
===========================================
- Hits         10721    10622      -99     
  Misses          37       37              
Impacted Files Coverage Δ
src/modules/notification.js 100.00% <100.00%> (ø)
src/services/investment/index.js 98.17% <100.00%> (+0.14%) ⬆️
src/models/pg/token.js 100.00% <0.00%> (ø)
src/models/pg/wallet.js 100.00% <0.00%> (ø)
src/helpers/appsFlyer.js 100.00% <0.00%> (ø)
src/modules/appsFlyer.js 100.00% <0.00%> (ø)
src/services/api/routers/wallets.js 100.00% <0.00%> (ø)
src/services/api/controllers/users.js 100.00% <0.00%> (ø)
src/services/api/controllers/wallets.js 100.00% <0.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48ab175...8411c52. Read the comment docs.

Comment thread src/modules/notification.js
Comment thread src/modules/notification.js
Comment thread src/modules/notification.js
NEED_CONNECTIONS: ['postgre', 'redis', 'coins', 'special-wallet'],

repeaters: {},
aggregateNotifications: [],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I got the logic, but:
This is fucking ugly
This is not scalable: I guess even with the low users we have now the process will crash by trying to keep all this data in memory. The best way for me seems to be after interest have been distributed make a second crawl to find all user that received at least one interest coming for this new interest rate list, then trigger email push notification. Will be nice to add global summary in euro 'This week you've received xxEur interest'.
I'm ok to try to accomplish this, I fear it will not work because sometime we do not received all interest for all currency in one shot, like we received some currency in first shot, then few hours later some other batch of currency.... We can't do much with this.... Then it will trigger twice the email summary

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yep you right its not scalable, I will improve it

@cristianizzo cristianizzo self-assigned this Jan 27, 2022
@julesGoullee
Copy link
Copy Markdown
Contributor

@cristianizzo this branch has conflict since I've merged the investment improvement

1 similar comment
@julesGoullee
Copy link
Copy Markdown
Contributor

@cristianizzo this branch has conflict since I've merged the investment improvement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants