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 blocked and deleted users from participants stats #8147

Conversation

alecslupu
Copy link
Contributor

@alecslupu alecslupu commented Jun 18, 2021

🎩 What? Why?

On the currents state of the application all the users are being added to the participant count, regardless the user is banned or deleted. This PR aims to fix the issue.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

♥️ Thank you!

@alecslupu alecslupu force-pushed the feature/exclude-blocked-deleted-participants-stats branch from 1a49260 to 1717c08 Compare June 18, 2021 15:53
@andreslucena
Copy link
Member

andreslucena commented Jul 12, 2021

Hi @alecslupu
I think this is a much-needed fix 🙌🏽
Is this still a draft? Can you provide some descriptions 🙏🏽?

@alecslupu alecslupu marked this pull request as ready for review July 12, 2021 11:46
@alecslupu
Copy link
Contributor Author

@andreslucena The description has been updated.

@leio10 this PR is ready to be reviewed.

@alecslupu
Copy link
Contributor Author

@carolromero , @andreslucena This can be tested on http://alecslupu.go.ro:3309/ (default decidim admin credentials)

@carolromero
Copy link
Member

checked ✔️, ready for technical review

Copy link
Contributor

@leio10 leio10 left a comment

Choose a reason for hiding this comment

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

Nice job, @alecslupu!! 👏👏

Just one request before merging it: Can you add a warning to the CHANGELOG about this change on the stats calculations? Those numbers are going to change after the update and it could alarm organizations, so we should let them know about it.

@alecslupu
Copy link
Contributor Author

@leio10 the following line has been added to the change log:

As per [\#8147](https://github.com/decidim/decidim/pull/8147), the participants stats will not take into account deleted and blocked users. 

@leio10 leio10 merged commit f722307 into decidim:develop Jul 19, 2021
@alecslupu alecslupu deleted the feature/exclude-blocked-deleted-participants-stats branch September 30, 2021 16:49
@andreslucena andreslucena added type: change PRs that implement a change for an existing feature and removed type: enhancement labels Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-review module: core type: change PRs that implement a change for an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants