Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
| if (options?.endOfDay) { | ||
| date.setUTCHours(23, 59, 59, 0); | ||
| } |
There was a problem hiding this comment.
The end-of-day timestamp is incomplete and will miss events in the last 999 milliseconds of the day. Line 113 sets milliseconds to 0 instead of 999, meaning until dates will exclude events between 23:59:59.001 and 23:59:59.999.
if (options?.endOfDay) {
date.setUTCHours(23, 59, 59, 999);
}| if (options?.endOfDay) { | |
| date.setUTCHours(23, 59, 59, 0); | |
| } | |
| if (options?.endOfDay) { | |
| date.setUTCHours(23, 59, 59, 999); | |
| } |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
| let since = query.since.as_ref().and_then(|s| { | ||
| DateTime::parse_from_rfc3339(s) | ||
| .ok() | ||
| .map(|dt| dt.with_timezone(&Utc).naive_utc()) | ||
| }); | ||
|
|
||
| let until = query.until.as_ref().and_then(|u| { | ||
| DateTime::parse_from_rfc3339(u) | ||
| .ok() | ||
| .map(|dt| dt.with_timezone(&Utc).naive_utc()) | ||
| }); |
There was a problem hiding this comment.
Timestamp parsing inconsistency causes different error handling behavior between endpoints. The get_drop_lands endpoint uses .and_then() which silently ignores invalid timestamp formats (treats them as None), while get_global_metrics (lines 232-240) returns BAD_REQUEST for the same scenario. This inconsistency will confuse API consumers.
// Change from and_then to map for consistent error handling
let since = query.since.as_ref().map(|s| {
DateTime::parse_from_rfc3339(s)
.ok()
.map(|dt| dt.with_timezone(&Utc).naive_utc())
});
// Then add validation like in get_global_metrics
let since = match since {
Some(Some(dt)) => Some(dt),
Some(None) => return Err(axum::http::StatusCode::BAD_REQUEST),
None => None,
};| let since = query.since.as_ref().and_then(|s| { | |
| DateTime::parse_from_rfc3339(s) | |
| .ok() | |
| .map(|dt| dt.with_timezone(&Utc).naive_utc()) | |
| }); | |
| let until = query.until.as_ref().and_then(|u| { | |
| DateTime::parse_from_rfc3339(u) | |
| .ok() | |
| .map(|dt| dt.with_timezone(&Utc).naive_utc()) | |
| }); | |
| let since = query.since.as_ref().map(|s| { | |
| DateTime::parse_from_rfc3339(s) | |
| .ok() | |
| .map(|dt| dt.with_timezone(&Utc).naive_utc()) | |
| }); | |
| let since = match since { | |
| Some(Some(dt)) => Some(dt), | |
| Some(None) => return Err(axum::http::StatusCode::BAD_REQUEST), | |
| None => None, | |
| }; | |
| let until = query.until.as_ref().map(|u| { | |
| DateTime::parse_from_rfc3339(u) | |
| .ok() | |
| .map(|dt| dt.with_timezone(&Utc).naive_utc()) | |
| }); | |
| let until = match until { | |
| Some(Some(dt)) => Some(dt), | |
| Some(None) => return Err(axum::http::StatusCode::BAD_REQUEST), | |
| None => None, | |
| }; |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
Deployment informationsThis PR has passed automatic testing, and is ready to be tested manually. Warning You might need to have a logged-in employee runelabs account to access the deployment environment.
This comment was written by a bot! |
knownasred
left a comment
There was a problem hiding this comment.
What is this. Please take a bit of time to read the comments, and address them before going further on this.
| if ( | ||
| hasBypassToken && | ||
| pathname.startsWith('/dashboard/drops') && | ||
| !hasValidBypass | ||
| ) { | ||
| return redirect(302, '/maintenance'); | ||
| } |
There was a problem hiding this comment.
If you wanted the drops dashboard to be private, then it would have been best to move it to xperience or another private repo. People can and will run this locally
| function parseDateValue(value: string): DateParts | null { | ||
| if (!value) return null; | ||
|
|
||
| const isoMatch = value.match(/^(\d{4})-(\d{2})-(\d{2})$/); | ||
| if (isoMatch) { | ||
| return { | ||
| year: Number(isoMatch[1]), | ||
| month: Number(isoMatch[2]), | ||
| day: Number(isoMatch[3]), | ||
| }; | ||
| } | ||
|
|
||
| const slashMatch = value.match(/^(\d{1,2})\/(\d{1,2})\/(\d{4})$/); | ||
| if (slashMatch) { | ||
| const first = Number(slashMatch[1]); | ||
| const second = Number(slashMatch[2]); | ||
| const year = Number(slashMatch[3]); | ||
|
|
||
| let month = first; | ||
| let day = second; | ||
| if (first > 12 && second <= 12) { | ||
| // First number cannot be month -> interpret as day/month | ||
| day = first; | ||
| month = second; | ||
| } else if (second > 12 && first <= 12) { | ||
| // Second number cannot be month -> keep default month/day order | ||
| day = second; | ||
| month = first; | ||
| } | ||
|
|
||
| if (month >= 1 && month <= 12 && day >= 1 && day <= 31) { | ||
| return { year, month, day }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid doing custom date parsing if possible. I don't see why it would be required in this specific case, as it is pretty much standard parsing
|
|
||
| async function parseResponse<T>(response: Response) { | ||
| if (!response.ok) { | ||
| throw new Error('Ocurrió un error al cargar las drops'); |
There was a problem hiding this comment.
Errors should be in english.
| async function parseResponse<T>(response: Response) { | ||
| if (!response.ok) { | ||
| throw new Error('Ocurrió un error al cargar las drops'); | ||
| } | ||
| return (await response.json()) as T; | ||
| } |
There was a problem hiding this comment.
Also, not sure it is useful, as you can type the return variable to the same effect, making things a bit easier to understand
| const DEFAULT_LEVEL = 1; | ||
| const DEFAULT_FEE_RATE = 900_000; | ||
| const DEFAULT_SALE_FEE_RATE = 500_000; |
There was a problem hiding this comment.
Please centralize them somewhere as game params, as I'm starting to see a lot of consts everywhere for a lot of logic
| let since = match since { | ||
| Some(Some(dt)) => Some(dt), | ||
| Some(None) => return Err(axum::http::StatusCode::BAD_REQUEST), | ||
| None => None, | ||
| }; |
There was a problem hiding this comment.
You can treat a Result instead? Why go through a map via .ok() ?
| .map_err(|_| axum::http::StatusCode::INTERNAL_SERVER_ERROR)?; | ||
|
|
||
| // Sum area protocol fees across all tokens | ||
| use sqlx::types::BigDecimal; |
| let total_revenue_bd: BigDecimal = metrics | ||
| .area_fees_per_token | ||
| .0 | ||
| .values() | ||
| .fold(zero_bd.clone(), |acc, v| acc + BigDecimal::from(*v)); | ||
| let total_inflows_bd: BigDecimal = metrics | ||
| .inflows_per_token | ||
| .0 | ||
| .values() | ||
| .fold(zero_bd.clone(), |acc, v| acc + BigDecimal::from(*v)); | ||
| let total_sale_fees_bd: BigDecimal = metrics | ||
| .sale_fees_per_token | ||
| .0 | ||
| .values() | ||
| .fold(zero_bd.clone(), |acc, v| acc + BigDecimal::from(*v)); |
There was a problem hiding this comment.
Sum through the U256 add
| // Calculate ROI | ||
| let distributed_bd = BigDecimal::from(metrics.total_distributed); | ||
| let global_roi = if distributed_bd > zero_bd { | ||
| let ratio = &total_revenue_bd / &distributed_bd; | ||
| ratio.to_string().parse::<f64>().unwrap_or(0.0) | ||
| } else { | ||
| 0.0 | ||
| }; | ||
|
|
||
| // Build per-token aggregates so the client can apply decimals/prices per token | ||
| let mut token_keys = std::collections::HashSet::new(); | ||
| for k in metrics.distributed_per_token.0.keys() { | ||
| token_keys.insert(k.clone()); | ||
| } | ||
| for k in metrics.area_fees_per_token.0.keys() { | ||
| token_keys.insert(k.clone()); | ||
| } | ||
| for k in metrics.inflows_per_token.0.keys() { | ||
| token_keys.insert(k.clone()); | ||
| } | ||
| for k in metrics.sale_fees_per_token.0.keys() { | ||
| token_keys.insert(k.clone()); | ||
| } |
There was a problem hiding this comment.
All of these seems to be logic that should go into a service. the API should only call the service, and do minimal parsing + re-formatting into the final format
| .map(|token| GlobalTokenMetricsResponse { | ||
| token: token.clone(), | ||
| distributed: metrics | ||
| .distributed_per_token | ||
| .0 | ||
| .get(&token) | ||
| .cloned() | ||
| .unwrap_or_else(|| sqlx::types::BigDecimal::from(0i32).into()) | ||
| .to_string(), | ||
| fees: metrics | ||
| .area_fees_per_token | ||
| .0 | ||
| .get(&token) | ||
| .cloned() | ||
| .unwrap_or_else(|| sqlx::types::BigDecimal::from(0i32).into()) | ||
| .to_string(), | ||
| sale_fees: metrics | ||
| .sale_fees_per_token | ||
| .0 | ||
| .get(&token) | ||
| .cloned() | ||
| .unwrap_or_else(|| sqlx::types::BigDecimal::from(0i32).into()) | ||
| .to_string(), | ||
| inflows: metrics | ||
| .inflows_per_token | ||
| .0 | ||
| .get(&token) | ||
| .cloned() | ||
| .unwrap_or_else(|| sqlx::types::BigDecimal::from(0i32).into()) | ||
| .to_string(), |
There was a problem hiding this comment.
No logic inside of a struct declaration, especially this much
757fb6b to
75bbf85
Compare

Add drop land dashboard with metrics tracking
This PR adds a new dashboard for tracking drop land metrics, including:
The implementation includes:
The dashboard provides comprehensive analytics for drop lands, helping users track performance and ROI across their reinjections.