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(website): Only count latest versions for stats on main page #2814

Merged
merged 10 commits into from
Sep 17, 2024

Conversation

theosanderson
Copy link
Member

@theosanderson theosanderson commented Sep 17, 2024

@theosanderson theosanderson added the preview Triggers a deployment to argocd label Sep 17, 2024
@theosanderson theosanderson added the format_me Triggers github_actions to format website code on PR label Sep 17, 2024
@theosanderson theosanderson changed the title Only count latest versions for stats on main page feat(website): Only count latest versions for stats on main page Sep 17, 2024
@Taepper
Copy link

Taepper commented Sep 17, 2024

How about changing the '+ count' to the following:
Number of accessions that had any new version added within the last 30 days. -> this would mean that we do not count multiple versions for the same accession multiple times

Nevermind, I missed that getRecent only counts version 1s anyways

@theosanderson
Copy link
Member Author

theosanderson commented Sep 17, 2024

I tested that this is working and I now believe there are (almost) no edge cases 🥳

@theosanderson theosanderson merged commit 096b1d2 into main Sep 17, 2024
19 checks passed
@theosanderson theosanderson deleted the Only-count-latest-versions-for-stats branch September 17, 2024 15:18
@@ -49,7 +50,8 @@ const getTotalAndLastUpdatedAt = async (
const client = LapisClient.createForOrganism(organism);
return (
await client.call('aggregated', {
version: 1,
[VERSION_STATUS_FIELD]: siloVersionStatuses.latestVersion,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should rename the function to getTotalUnrevokedAndLastUpdatedAt

Copy link
Member Author

Choose a reason for hiding this comment

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

Personally I think this is ok as is (but I can live with the other option too)

@@ -67,12 +69,22 @@ const getTotalAndLastUpdatedAt = async (
const getRecent = async (organism: string, numberDaysAgo: number): Promise<number> => {
const recentTimestamp = Math.floor(Date.now() / 1000 - numberDaysAgo * 24 * 60 * 60);
const client = LapisClient.createForOrganism(organism);
return (
const recentTotal = (
Copy link
Contributor

Choose a reason for hiding this comment

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

recentTotalIncludingRevoked

corneliusroemer added a commit that referenced this pull request Sep 17, 2024
Also some slight tweaks to function names in organism statistics, follow up to #2814
corneliusroemer added a commit that referenced this pull request Sep 17, 2024
* refactor(website): rename `siloVersionStatus` to `versionStatus`

Also some slight tweaks to function names in organism statistics, follow up to #2814

Coauthered by Theo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format_me Triggers github_actions to format website code on PR preview Triggers a deployment to argocd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants