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.

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

feat: handle analytics data after expiry #202

Merged
merged 5 commits into from
Apr 25, 2024
Merged

Conversation

ayushtom
Copy link
Contributor

@ayushtom ayushtom commented Mar 20, 2024

We need to stop using data for analytics for tasks completion of the quest once the quest has expired. This data is irrelevant and we would need to remove it from the stats dashboard.

We also modify the document model for to add some kind of automation on quests -
https://www.notion.so/lfg-labs/Quest-DB-Update-6a216557a4084a60a5cd744f07c8a92a?pvs=4

@ayushtom ayushtom added the 🚧 In progress do not merge Pull Request in progress, please do not merge label Mar 20, 2024
@ayushtom ayushtom self-assigned this Mar 20, 2024
@ayushtom ayushtom force-pushed the ayush/handle-expiry-data branch from f2211a5 to 25bc1ed Compare April 18, 2024 09:34
@ayushtom ayushtom force-pushed the ayush/handle-expiry-data branch from 25bc1ed to 95a7192 Compare April 18, 2024 09:35
@ayushtom ayushtom added 🔥 Ready for review This pull request needs a review and removed 🚧 In progress do not merge Pull Request in progress, please do not merge labels Apr 19, 2024
@Th0rgal Th0rgal self-requested a review April 23, 2024 16:34
Copy link
Member

@Th0rgal Th0rgal left a comment

Choose a reason for hiding this comment

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

Actually it lgtm, I am just used of storing the unix timestamp (in seconds). Using millis is fine as long as it is expected. Any reason why you preferred millis over seconds?

)]
pub async fn handler(
State(state): State<Arc<AppState>>,
Query(query): Query<GetQuestsQuery>,
) -> impl IntoResponse {
let current_time = chrono::Utc::now().timestamp_millis();
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't it be in seconds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I personally make it miliseconds to make it precise in all cases. Not that it's very relevant for our usecase but i followed this convention in other endpoints like tasks and boosts too.Do you think i should change this to seconds?

@Th0rgal Th0rgal added ❌ Change request Change requested from reviewer and removed 🔥 Ready for review This pull request needs a review labels Apr 24, 2024
@Th0rgal
Copy link
Member

Th0rgal commented Apr 24, 2024

Probabyl doesn't require a change but just an explanation :)

@ayushtom ayushtom requested a review from Th0rgal April 25, 2024 10:22
Copy link
Member

@Th0rgal Th0rgal left a comment

Choose a reason for hiding this comment

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

lgtm

@Th0rgal Th0rgal merged commit c006acd into testnet Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
❌ Change request Change requested from reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants