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

Exclude federated and app users from active user count #16489

Merged
merged 3 commits into from
Feb 7, 2020

Conversation

d-gubert
Copy link
Member

@d-gubert d-gubert commented Feb 5, 2020

This PR makes the active user count of the statistics stop taking app and federated users into consideration

Before
image

After
image

Criteria for active users
image

Things to consider:

  • Do we want the count of app + federated users to go to the Deactivated users count?
  • Should we add App users and Federated users counts?

statistics.totalUsers = Meteor.users.find().count();
statistics.activeUsers = Meteor.users.find({ active: true }).count();
statistics.totalUsers = Users.find().count();
statistics.activeUsers = Users.getActiveLocalUserCount();
Copy link
Contributor

@geekgonecrazy geekgonecrazy Feb 5, 2020

Choose a reason for hiding this comment

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

This is so it excludes remote and app users right?

Can we add app users like we have with federated users below? Would be probably useful to know how many app users specifically like we have with remote users

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think we should add the count of app users

@geekgonecrazy
Copy link
Contributor

Do we want the count of app + federated users to go to the Deactivated users count

I say no. Because they aren't deactivated they just aren't considered active users

Should we add App users and Federated users counts?

Yes to app users! We already have federated user count

@d-gubert
Copy link
Member Author

d-gubert commented Feb 6, 2020

image

There is already one item of the total count of apps on the server... do you think it is still useful to know the count of app users? @geekgonecrazy @rodrigok

@d-gubert d-gubert added this to the 3.0.0 milestone Feb 6, 2020
@rodrigok
Copy link
Member

rodrigok commented Feb 6, 2020

@d-gubert I think so, it's directly related right now but this may change in the future.

@geekgonecrazy
Copy link
Contributor

geekgonecrazy commented Feb 6, 2020

Agreed! I think its still useful to be able to make sense of the user totals.

@sampaiodiego sampaiodiego merged commit e2972c8 into develop Feb 7, 2020
@sampaiodiego sampaiodiego deleted the user-count-statistics branch February 7, 2020 01:46
gabriellsh added a commit to ritwizsinha/Rocket.Chat that referenced this pull request Feb 13, 2020
…5997-ritwizsinha-15996

* 'develop' of github.com:RocketChat/Rocket.Chat: (181 commits)
  Update Livechat widget dependency version to 1.3.1. (RocketChat#16580)
  Update Apps-Engine version (RocketChat#16584)
  [FIX] Error when successfully joining room by invite link (RocketChat#16571)
  Add breaking notice regarding TLS (RocketChat#16575)
  [FIX] Invite links proxy URLs not working when using CDN (RocketChat#16581)
  Regression: Modal onSubmit (RocketChat#16556)
  Regression: UIkit input states (RocketChat#16552)
  [FIX] Do not stop on DM imports if one of users was not found (RocketChat#16547)
  [FIX] Introduce AppLivechatBridge.isOnlineAsync method (RocketChat#16467)
  Regression: UIKit missing select states: error/disabled (RocketChat#16540)
  [BREAK] Change apps/icon endpoint to return app's icon and use it to show on Ui Kit modal (RocketChat#16522)
  Regression: update package-lock (RocketChat#16528)
  Regression: Update Uikit (RocketChat#16515)
  Regression: UIKit - Send container info on block actions triggered on a message (RocketChat#16514)
  Use base64 for import files upload to prevent file corruption (RocketChat#16516)
  Regression: Ui Kit messaging issues RocketChat#16513
  Regression: Send app info along with interaction payload to the UI (RocketChat#16511)
  Fix: License missing from manual register handler (RocketChat#16505)
  Exclude federated and app users from active user count (RocketChat#16489)
  Remove users.info being called without need (RocketChat#16504)
  ...
@sampaiodiego sampaiodiego mentioned this pull request Feb 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants