-
Notifications
You must be signed in to change notification settings - Fork 14
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
[Dashboard] Weekly/monthly funds trend #3080
base: feat/enhanced-dashboard
Are you sure you want to change the base?
Conversation
517627b
to
087fcd2
Compare
d9a379c
to
912e713
Compare
912e713
to
debbdbb
Compare
lastUpdatedAt?: Date; | ||
} | ||
|
||
export const useCurrencyConversionRate = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to update this to take historical data into account is date is provided
b20c0e6
to
e5b85e8
Compare
debbdbb
to
4dc8b05
Compare
e5b85e8
to
d125286
Compare
136167f
to
bc47745
Compare
9fdf423
to
b8cf268
Compare
1400d51
to
cb08bb7
Compare
f0e9fb9
to
6b6ca2a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
D A M N
took me a while to get through everything, but this stuff is actually quite straight forward. Nice job with the caching per day, basically saving us a ton of compute time for subsequent requests 👍
Left just some minor "cosmetic" comments which we can tackle, but overall a really great job with this!
So I can confirm that batch processing happens when triggering this lambda:
Filtering over teams works like a charm too:
It also doesn't break on mobile:
So I think we're good with this implementation, we probably need to run it against large sets of data to see how many resources it's gonna hog :D
TOTAL: "TOTAL" | ||
} | ||
|
||
const getYearsFromNow = (numberOfYears) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe the naming of these helpers could be something like getPastYears since "from now" can go either into the future or into the past? or just something like subtractYearsFromNow
} = event.arguments?.input || {}; | ||
const periodFromNow = getPeriodFromNow(timeframePeriod, timeframeType); | ||
const periodFromNow = getPeriodFromNow(timeframePeriod, timeframeType, timeframePeriodEndDate); | ||
console.log('fetchDomainBalance', periodFromNow, timeframeType, timeframePeriodEndDate) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👀
|
||
return { | ||
value: `${trend.toString()}%`, | ||
isIncrease: trend.gte(0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's probably no realistic chance for this to happen, but should we just return percentages for value
and if it's 0
don't display the trend at all?
d10cf57
to
4476c6a
Compare
Feat: Add cron lambda to cache all needed previous domain balances
Feat: Add payments and income trends Update cacheDomainBalance cron lambda
Process balance requests in batches Rename requests.js into operations.js
…data Refactor: Update previousBalance to be converted to token historical data
Refactor: Changes after testing historical data
Refactor: Align after rebase
Refactor: Add missing condition for 0 percent trend
Fix: Align after rebase
6b6ca2a
to
e874e26
Compare
Feat: Add cron lambda to cache all needed previous domain balances
Description
This PR adds a cron lambda to cache all previous domain-level and colony-level balances for both the previous last 30 days and the previous total amount from the start of last week. Given the time at which the lambda is triggered, we have no idea what the selected currency is and we want to provide a uniform solution, the cached value will be in
USDC
These values will be used for the increase/decrease trends in these cards, however this PR fetches the value only for the
Total in and out
cardTesting
TODO: Given you can't really test the cron lambda runs properly locally, as there is no event to trigger it, we need to make use of a manual query
create-data
script.diff
)The reason for this patch is to be able to manipulate the
updatedAt
field for a colony actiongit apply .diff
Go to http://localhost:20002 and run the following mutation
Total in and out
cardTo be sure, the values are properly computed check the
getValuesTrend
methodThe formula is
If current === 0
-100
if current !== 0 and previous === 0
100
if previous > 0 and current > 0
(previous - current) * 100 / previous
- special thanks to @Nortsova 🥇Further testing: please be as creative as possible
Diffs
New stuff ✨
cacheDomainBalance
lambdauseCurrencyHistoricalConversionRate
hook to compute the USDC value to the selected currency based on the date at which the historical balance was computedgetValuesTrend
utilsGetCachedDomainBalance
queryChanges 🏗
previousValue
property toTitledSection
componentTODO
Resolves #2858