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.

Sign up for GitHub

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

Jump to bottom

Only include APPROVED users for stats on homepage #1952

Merged
merged 10 commits into from
Sep 20, 2024
Merged

Only include APPROVED users for stats on homepage #1952

merged 10 commits into from
Sep 20, 2024

Conversation

aaronkanzer
Copy link
Member

Cc @kabilar -- per our fix in LINC

The current query on the landing page that shows stats does not filter by user type (e.g. it takes in account REJECTED uers)

This PR includes the proper query.

@kabilar
Copy link
Member

kabilar commented Jun 4, 2024

Thank you, @aaronkanzer.

Hi @waxlamp, we noticed this behavior on lincbrain.org where the users count on the homepage included those users with a STATUS of APPROVED, PENDING, INCOMPLETE and REJECTED. Presumably the desired behavior is only to count the users who are APPROVED.

@kabilar kabilar requested a review from waxlamp June 4, 2024 03:11
waxlamp
waxlamp requested changes Aug 22, 2024
View reviewed changes
dandiapi/api/tests/test_stats.py Outdated Show resolved Hide resolved
dandiapi/api/tests/test_stats.py Outdated Show resolved Hide resolved
dandiapi/api/tests/test_stats.py Outdated Show resolved Hide resolved
dandiapi/api/models/asset.py Outdated Show resolved Hide resolved
@yarikoptic
Copy link
Member

@aaronkanzer what is your plans for this PR?

@waxlamp waxlamp self-assigned this Sep 10, 2024
@kabilar
Copy link
Member

kabilar commented Sep 11, 2024

@aaronkanzer what is your plans for this PR?

Sorry, @yarikoptic, not sure what you mean. After discussion with @aaronkanzer, he will be coming back to these PRs over the next few weeks.

@aaronkanzer
Copy link
Member Author

@waxlamp thanks for the review -- I've updated the code accordingly to your review.

Cc @kabilar @yarikoptic

mvandenburgh
mvandenburgh requested changes Sep 16, 2024
View reviewed changes
dandiapi/api/tests/test_stats.py Outdated Show resolved Hide resolved
dandiapi/api/tests/test_stats.py Outdated Show resolved Hide resolved
waxlamp and others added 2 commits September 19, 2024 20:20
@waxlamp @mvandenburgh
Avoid deleting user objects explicitly
f42558a 
This is not needed due to the testing framework creating a fresh database for each test run.

Co-authored-by: Mike VanDenburgh <37340715+mvandenburgh@users.noreply.github.com>
@waxlamp @mvandenburgh
Simplify test code
a8b5bd6 
Co-authored-by: Mike VanDenburgh <37340715+mvandenburgh@users.noreply.github.com>
@waxlamp waxlamp added patch Increment the patch version when merged release Create a release when this pr is merged labels Sep 20, 2024
@waxlamp waxlamp merged commit 7409e26 into dandi:master Sep 20, 2024
11 checks passed
@dandibot
Copy link
Member

🚀 PR was released in v0.3.101 🚀

@dandibot dandibot added the released This issue/pull request has been released. label Sep 20, 2024
@kabilar
Copy link
Member

kabilar commented Sep 23, 2024

Thanks team. Based on these changes it looks like the number of users listed on the homepage (https://dandiarchive.org/) went from 1908 to 1412.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mvandenburgh mvandenburgh mvandenburgh requested changes

@waxlamp waxlamp waxlamp approved these changes

Assignees

@waxlamp waxlamp

Labels
patch Increment the patch version when merged release Create a release when this pr is merged released This issue/pull request has been released.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@aaronkanzer @kabilar @yarikoptic @dandibot @waxlamp @mvandenburgh