Skip to content

[bug] Monitoring stats: count only verified and active users #663

@nemesifier

Description

@nemesifier

⚠️ Warning: not suited for beginner contributors. PRs from beginner contributors will be closed without discussion.

The current monitoring stats which log new signups and total amount of users are counting also unverified users and inactive users.

I spent some time to change the queries and tests to fix it as it seemed an easy issue to solve, but I later realized that the solution would still be imperfect, due to the fact that the celery tasks run periodically and at the moment of running, users may be completing the verification process and may not be counted ever in the sign up stats.

I think we should change the implementation as follows.

Total user stats

  • Count only active users (filter by is_active=True, I was trying to tag along with the exclude logic in the current code but didn't seem to work well).
  • For users having a RegisteredUser relation, ensure they have is_verified=True, if identity verification is mandatory for that org

New registrations

This is a tricky one. I think we can avoid worrying about is_active=True because it's highly unlikely that a user signs up, completes the verification process and gets banned right away.

However, we shouldn't count unverified users here, but doing so in the periodic task could mean we would lose some users.
I propose that for users which have a RegisteredUser relation we stop using the periodic celery task and perform the timeseries DB write once Is_verified is set to True, which will require the addition of a new signal for this module (the docs only mentions the radius accounting signal).

We also need to take into account the case in which identity verification is not mandatory, which makes this even trickier, because it means we should count the user regardless of the is_verified flag: the old code could work, but keeping both implementations may make maintenance harder, a possible alternative solution would be to bind both signals: post_save and verification_success, using one or the other depending on whether identity verification is mandatory.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    Projects

    Status

    Backlog

    Status

    To do (general)

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions