Conversation
sshanzel
requested review from
AmarTrebinjac,
ilasw,
nensidosari,
omBratteng and
rebelchris
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bugbot free trial expires on July 31, 2025
Learn more in the Cursor dashboard.
Comment bugbot run to trigger another review on this PR
|
🍹 The Update (preview) for dailydotdev/api/prod (at 91856af) was successful. Resource Changes Name Type Operation
~ vpc-native-update-source-public-threshold-cron kubernetes:batch/v1:CronJob update
~ vpc-native-update-trending-cron kubernetes:batch/v1:CronJob update
~ vpc-native-sync-subscription-with-cio-cron kubernetes:batch/v1:CronJob update
~ vpc-native-update-highlighted-views-cron kubernetes:batch/v1:CronJob update
~ vpc-native-personalized-digest-cron kubernetes:batch/v1:CronJob update
~ vpc-native-private-deployment kubernetes:apps/v1:Deployment update
~ vpc-native-clean-zombie-images-cron kubernetes:batch/v1:CronJob update
~ vpc-native-clean-zombie-users-cron kubernetes:batch/v1:CronJob update
~ vpc-native-clean-stale-user-transactions-cron kubernetes:batch/v1:CronJob update
~ vpc-native-personalized-digest-deployment kubernetes:apps/v1:Deployment update
~ vpc-native-bg-deployment kubernetes:apps/v1:Deployment update
~ vpc-native-validate-active-users-cron kubernetes:batch/v1:CronJob update
~ vpc-native-update-source-tag-view-cron kubernetes:batch/v1:CronJob update
~ vpc-native-daily-digest-cron kubernetes:batch/v1:CronJob update
- vpc-native-api-migration-39fd11d5 kubernetes:batch/v1:Job delete
~ vpc-native-ws-deployment kubernetes:apps/v1:Deployment update
~ vpc-native-clean-gifted-plus-cron kubernetes:batch/v1:CronJob update
+ vpc-native-api-migration-46b91bf4 kubernetes:batch/v1:Job create
~ vpc-native-clean-zombie-user-companies-cron kubernetes:batch/v1:CronJob update
~ vpc-native-generate-search-invites-cron kubernetes:batch/v1:CronJob update
~ vpc-native-update-current-streak-cron kubernetes:batch/v1:CronJob update
~ vpc-native-generic-referral-reminder-cron kubernetes:batch/v1:CronJob update
~ vpc-native-hourly-notification-cron kubernetes:batch/v1:CronJob update
~ vpc-native-calculate-top-readers-cron kubernetes:batch/v1:CronJob update
~ vpc-native-deployment kubernetes:apps/v1:Deployment update
~ vpc-native-temporal-deployment kubernetes:apps/v1:Deployment update
~ vpc-native-check-analytics-report-cron kubernetes:batch/v1:CronJob update
~ vpc-native-update-tag-recommendations-cron kubernetes:batch/v1:CronJob update
~ vpc-native-update-tags-str-cron kubernetes:batch/v1:CronJob update
~ vpc-native-update-views-cron kubernetes:batch/v1:CronJob update
|
|
As promised, I also removed the typescript magic we had before: 6f7fe38 |
| }; | ||
|
|
||
| if (budget && durationInDays) { | ||
| params.duration = durationInDays * ONE_DAY_IN_SECONDS; |
There was a problem hiding this comment.
why not just accepts seconds on API and just say duration, then we can calculate how ever we want?
There was a problem hiding this comment.
I think it is better to keep it in days; most of the mutations understand that we need to send the amount in days, then we do the calculation here, so we will only do the conversion once.
There was a problem hiding this comment.
yeah but see how skadi accepts seconds, it is much more future proof
if you want to proceed with days, ok, but let's make the separate method for estimatePostBoostReachDaily as you did for mutation.
There was a problem hiding this comment.
Sure, we can do that.
There was a problem hiding this comment.
it can even call base method which would accept duration in seconds (to dedupe the logic for fetch)
There was a problem hiding this comment.
Good point. Let me add that.
capJavert
left a comment
capJavert
left a comment
There was a problem hiding this comment.
Just some sanity checks, thanks for taking care of camel case helper and proper tests, awesome effort!
2 participants
The API for estimated reach can now accept
budgetanddurationoptionally.When those parameters are present, we will send back a different calculation compared to the original one.
Note: the diff on the test file is huge. Rather than mocking the api client, I am mocking the request directly to have a better look of what we are sending and what we are receiving.
Jira ticket
MI-961