-
-
Notifications
You must be signed in to change notification settings - Fork 399
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
Remove show statistics boolean fields in assemblies and processes #13123
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pull request does not contain a valid label. Please add one of the following labels: ['type: feature', 'type: change', 'type: fix', 'type: removal', 'target: developer-experience', 'type: internal']
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I found more places, can you check them out please?
$ rg -l show_statistic decidim-{assemblies,participatory_processes}/{app/,config/locales/en.yml,lib/,spec/}
decidim-assemblies/config/locales/en.yml
decidim-participatory_processes/config/locales/en.yml
decidim-participatory_processes/spec/commands/update_participatory_process_spec.rb
decidim-assemblies/lib/decidim/assemblies/test/factories.rb
decidim-participatory_processes/lib/decidim/participatory_processes/test/factories.rb
decidim-assemblies/lib/decidim/api/assembly_type.rb
decidim-participatory_processes/lib/decidim/api/participatory_process_type.rb
decidim-assemblies/spec/commands/update_assembly_spec.rb
decidim-assemblies/spec/forms/assembly_form_spec.rb
decidim-assemblies/spec/system/homepage_content_blocks_spec.rb
decidim-assemblies/spec/system/assemblies_spec.rb
decidim-assemblies/app/presenters/decidim/assemblies/admin_log/assembly_presenter.rb
decidim-participatory_processes/app/serializers/decidim/participatory_processes/participatory_process_importer.rb
decidim-assemblies/spec/serializers/decidim/assemblies/assembly_serializer_spec.rb
decidim-participatory_processes/app/serializers/decidim/participatory_processes/participatory_process_serializer.rb
decidim-assemblies/spec/types/assembly_type_spec.rb
decidim-assemblies/spec/types/integration_schema_spec.rb
decidim-participatory_processes/app/controllers/decidim/participatory_processes/participatory_processes_controller.rb
decidim-participatory_processes/spec/controllers/participatory_processes_controller_spec.rb
decidim-participatory_processes/spec/system/homepage_content_blocks_spec.rb
decidim-participatory_processes/spec/serializers/decidim/participatory_processes/participatory_process_importer_spec.rb
decidim-assemblies/app/serializers/decidim/assemblies/assembly_serializer.rb
decidim-assemblies/app/serializers/decidim/assemblies/assembly_importer.rb
$ rg -l show_metrics decidim-{assemblies,participatory_processes}/{app/,config/locales/en.yml,lib/,spec/}
decidim-participatory_processes/config/locales/en.yml
decidim-participatory_processes/spec/commands/update_participatory_process_spec.rb
decidim-participatory_processes/lib/decidim/participatory_processes/test/factories.rb
decidim-participatory_processes/lib/decidim/api/participatory_process_type.rb
decidim-participatory_processes/spec/serializers/decidim/participatory_processes/participatory_process_importer_spec.rb
decidim-participatory_processes/app/serializers/decidim/participatory_processes/participatory_process_importer.rb
decidim-participatory_processes/app/serializers/decidim/participatory_processes/participatory_process_serializer.rb
…idim into chore/remove-show-statistics
The failing spec is related, can you check it out?
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found four other relevant files and I think we should be ready to merge:
./decidim-dev/lib/decidim/dev/assets/assemblies.json: "show_statistics": false,
./decidim-dev/lib/decidim/dev/assets/participatory_processes_with_null.json: "show_statistics": true,
./decidim-dev/lib/decidim/dev/assets/assemblies_with_null.json: "show_statistics": false,
./decidim-dev/lib/decidim/dev/assets/participatory_processes.json: "show_statistics": true,
Also I think this should be a type: removal
so we don't backport it (as it's actually not doing any harm to have this extra field here), and also so if any implementer is in doubt it's much easier to find in the Changelog.
In #13119, I propose that we first let implementers know that these extra useless fields are deprecated so we can remove them in the next release (v0.30). Can you add these fields here?
Line 56 in 6f96fcc
## 2.4 Removal of useless fields |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found one more place and I have a mini fix in the releases notes:
decidim/decidim-participatory_processes/lib/decidim/participatory_processes/test/factories.rb
Line 33 in cf28abe
show_metrics { true } |
Co-authored-by: Andrés Pereira de Lucena <[email protected]>
…3123) * Remove show statistics in assemblies * Remove show statistics in Participatory Process * fix specs * Remove show Metrics & clean up * Cleanup * Fix failing specs * Apply review recommendations * Apply review recommendation * Update RELEASE_NOTES.md Co-authored-by: Andrés Pereira de Lucena <[email protected]> --------- Co-authored-by: Andrés Pereira de Lucena <[email protected]>
🎩 What? Why?
Since Decidim 0.28, we have 2 n ew content blocks that we can configure for
assemblies
andparticipatory process
. If you enable those content blocks, you may get in a situation when you enable the content block, yet the stats to not be shown, as the admin needs to go in the edit assembly admin page, where you would need to enableShow statistics
Testing
Show statistics
, then saveshow statistics
Additionally, on Processes you would have the show metrics checkbox that has been removed.
📷 Screenshots
Please add screenshots of the changes you are proposing