Skip to content

Add comments in participatory space presentation page stats block#8034

Merged
leio10 merged 3 commits intodecidim:developfrom
i-need-another-coffee:feature/add-comments-stat-on-topic
Jun 15, 2021
Merged

Add comments in participatory space presentation page stats block#8034
leio10 merged 3 commits intodecidim:developfrom
i-need-another-coffee:feature/add-comments-stat-on-topic

Conversation

@roxanaopr
Copy link
Copy Markdown
Contributor

@roxanaopr roxanaopr commented May 21, 2021 •
edited
Loading

🎩 What? Why?

Add comments count for each topic on the About page, similar to the homepage statistics block.

📌 Meta Proposal

https://meta.decidim.org/processes/roadmap/f/122/proposals/16344

📌 Related Issues

Link your PR to an issue

  • Related to #?
  • Fixes #?

Testing

Describe the best way to test or validate your PR.

📋 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.

📷 Screenshots

Screenshot from 2021-05-26 14-27-53

♥️ Thank you!

@roxanaopr roxanaopr force-pushed the feature/add-comments-stat-on-topic branch from 1786cce to c281b3b Compare May 24, 2021 06:52
@roxanaopr roxanaopr marked this pull request as ready for review May 24, 2021 08:41
leio10
leio10 previously approved these changes May 26, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@leio10 leio10 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice implementation! 👏

I like the final approach, that doesn't add dependencies for any specific module in core. But I have some doubts before merging it:

  • The title and the related proposal refer to "participatory spaces", but you are only adding the stats to "participatory processes". Should we add it also to assemblies and other spaces?
  • I saw that proposals and budgets already have the comments_count stats registered, so I understand that you are adding the missing ones (meetings). But I also found that accountability and blogs also have the comments_count column. Should we register the stats for those too?
  • In the screenshot I see two "COMMENTS" stats. That's a bug? It was fixed after taking the screenshot or it's still there?

@roxanaopr
Copy link
Copy Markdown
Contributor Author

roxanaopr commented May 26, 2021

  • In the screenshot I see two "COMMENTS" stats. That's a bug? It was fixed after taking the screenshot or it's still there?

I've added the wrong screenshot, sorry. I've added the correct one :D In stats should be only one "Comments" stat

@roxanaopr
Copy link
Copy Markdown
Contributor Author

roxanaopr commented May 26, 2021 •
edited
Loading

  • The title and the related proposal refer to "participatory spaces", but you are only adding the stats to "participatory processes". Should we add it also to assemblies and other spaces?
  • I saw that proposals and budgets already have the comments_count stats registered, so I understand that you are adding the missing ones (meetings). But I also found that accountability and blogs also have the comments_count column. Should we register the stats for those too?

My assumption was that we should add that stat for participatory processes, but I think that we can also add it to other spaces. It will be nice to have all those spaces specified in the meta proposal, because it wasn't clear enough for me.

@leio10
Copy link
Copy Markdown
Contributor

leio10 commented May 26, 2021

@roxanaopr ok, maybe @decidim/product can help us with those questions

@carolromero
Copy link
Copy Markdown
Member

carolromero commented Jun 7, 2021

Hi there! Sorry for the delay @roxanaopr and @leio10.

Regarding your doubts:

The title and the related proposal refer to "participatory spaces", but you are only adding the stats to "participatory processes". Should we add it also to assemblies and other spaces?

in the proposal it says participatory spaces, so we assume that it refers to all spaces that have statistics, that is: Processes, Assemblies and Conferences.

I saw that proposals and budgets already have the comments_count stats registered, so I understand that you are adding the missing ones (meetings). But I also found that accountability and blogs also have the comments_count column. Should we register the stats for those too?

Yep, we should count all comments from all components that make use of them.

@roxanaopr do you think it is possible to add it, in this or another PR? Thank you!

@roxanaopr
Copy link
Copy Markdown
Contributor Author

roxanaopr commented Jun 7, 2021

Hello @carolromero! Thank you for the response! I will add in this PR the remaining implementation.

@alecslupu
Copy link
Copy Markdown
Contributor

alecslupu commented Jun 7, 2021

@carolromero I guess it would be great if you could raise a meta proposal and make sure that all the metrics / stats are aligned between all the participatory spaces.

@alecslupu alecslupu force-pushed the feature/add-comments-stat-on-topic branch 2 times, most recently from bf64cc6 to 227bcf6 Compare June 10, 2021 21:55
@roxanaopr
Add comments_count statistic on participatory spaces: processes, proc…
ac1448d 
…ess groups, assemblies, conferences, votings
@roxanaopr roxanaopr force-pushed the feature/add-comments-stat-on-topic branch from 227bcf6 to ac1448d Compare June 11, 2021 06:54
leio10
leio10 previously approved these changes Jun 11, 2021
View reviewed changes
Copy link
Copy Markdown
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.

LGTM, please let me know your opinion about my doubts.

@leio10 leio10 closed this Jun 15, 2021
@leio10 leio10 reopened this Jun 15, 2021
@leio10 leio10 merged commit b971cff into decidim:develop Jun 15, 2021
entantoencuanto added a commit that referenced this pull request Jun 29, 2021
@entantoencuanto
Merge branch 'develop' into active_storage_migration
13dd1f4 
* develop: (47 commits)
  New Crowdin updates (#8150)
  Move the webpacker config override to @decidim/webpacker (#8158)
  Fix admin stylesheet dynamic imports (#8154)
  Fix session timeout conflicting with remember me (#7467)
  Allow to create online meetings without an URL (#8152)
  Fix verification route issues (#8146)
  Fix dont save timeout path to session (#8142)
  Fix access to import CSV results in accountability (#8132)
  Fix user report notification reported user name (#8130)
  Allow users to comment and delete their own comments (#8072)
  New Crowdin updates (#8124)
  Fix webpacker issues (#8136)
  Add comments in participatory space presentation page stats block (#8034)
  Split NPM dependencies to more granular packages (#8121)
  Metric is not shown when value is zero for blocked and reported users (#8117)
  Add missing templates translations (#8133)
  Add missing translation for authorization_modals (#8129)
  Polls in meetings (#8065)
  Fix flaky test on initiatives (#8128)
  Filter participants admin (#8104)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@leio10 leio10 leio10 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

in-review module: participatory processes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@roxanaopr @leio10 @carolromero @alecslupu