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.

Sign up for GitHub

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

Jump to bottom

Remove description from questions in elections #9401

Merged
merged 7 commits into from
Jun 16, 2022
Merged

Remove description from questions in elections #9401

merged 7 commits into from
Jun 16, 2022

Conversation

microstudi
Copy link
Contributor

@microstudi microstudi commented Jun 8, 2022 •
edited
Loading

🎩 What? Why?

Description is not used at all in elections UI.
This PR removes the description but still sends to the bulletin board and empty "subtitle" for the question just to keep compatibility. This should ease things up if one day description must be reinstated.

  • Remove description
  • Check bulletin when no descrption is sent

📌 Related Issues

Link your PR to an issue

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

Please add screenshots of the changes you're proposing
Description

♥️ Thank you!

@microstudi microstudi marked this pull request as ready for review June 9, 2022 16:25
@microstudi
Copy link
Contributor Author

This should be ready for review @ahukkanen @andreslucena

@andreslucena andreslucena added contract: e-voting Barcelona City Council contract module: elections type: fix PRs that implement a fix for a bug labels Jun 14, 2022
ahukkanen
ahukkanen requested changes Jun 14, 2022
View reviewed changes
Copy link
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

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

LGTM overall.

@microstudi Is it on purpose you are still preserving the description field in the database? Maybe we should remove it from the DB as well if we want to get rid of that field. But maybe you have a valid reason to preserve it?

Another place where it is still preserved is the translations:

decidim/decidim-elections/config/locales/en.yml

Line 16 in a45e747

description: Description

@microstudi
Copy link
Contributor Author

Thanks for your sharp eye @ahukkanen !
I've added a migration and removed the i18n key

@ahukkanen
Copy link
Contributor

I've added a migration and removed the i18n key

Great thanks @microstudi !

But note that there was one spec that was calling the description method for the question which is now broken. Could you take a look at that please?

@microstudi microstudi requested a review from ahukkanen June 15, 2022 14:36
@microstudi
Copy link
Contributor Author

all green @ahukkanen !

ahukkanen
ahukkanen approved these changes Jun 15, 2022
View reviewed changes
Copy link
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

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

LGTM

@ahukkanen ahukkanen requested a review from andreslucena June 16, 2022 14:22
andreslucena
andreslucena approved these changes Jun 16, 2022
View reviewed changes
Copy link
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏽

@andreslucena andreslucena merged commit 84f01fa into develop Jun 16, 2022
@andreslucena andreslucena deleted the fix/remove-question-description branch June 16, 2022 14:35
@ahukkanen ahukkanen mentioned this pull request Jun 16, 2022
Merged
12 tasks
andreslucena pushed a commit that referenced this pull request Jun 17, 2022
@microstudi @andreslucena
Remove description from questions in elections (#9401)
06fbbdb 
* remove description

* remove description from tests

* send empty description to keep bulletin board happy

* fix schema

* fix question type spec

* remove description from questions in the DB

* remove failing spec
andreslucena pushed a commit that referenced this pull request Jun 29, 2022
@microstudi @andreslucena
Remove description from questions in elections (#9401)
66bc042 
* remove description

* remove description from tests

* send empty description to keep bulletin board happy

* fix schema

* fix question type spec

* remove description from questions in the DB

* remove failing spec
andreslucena pushed a commit that referenced this pull request Jul 6, 2022
@microstudi @andreslucena
Remove description from questions in elections (#9401)
8d969a1 
* remove description

* remove description from tests

* send empty description to keep bulletin board happy

* fix schema

* fix question type spec

* remove description from questions in the DB

* remove failing spec
@andreslucena andreslucena mentioned this pull request Jul 6, 2022
Merged
andreslucena pushed a commit that referenced this pull request Jul 6, 2022
@microstudi @andreslucena
Remove description from questions in elections (#9401)
27a1fc3 
* remove description

* remove description from tests

* send empty description to keep bulletin board happy

* fix schema

* fix question type spec

* remove description from questions in the DB

* remove failing spec
ahukkanen pushed a commit that referenced this pull request Jul 8, 2022
@andreslucena @microstudi
Backport 'Remove description from questions in elections' to v0.26 (#…
65e824b 
…9522)

* Remove description from questions in elections (#9401)

* remove description

* remove description from tests

* send empty description to keep bulletin board happy

* fix schema

* fix question type spec

* remove description from questions in the DB

* remove failing spec

* Fix Rails version on migration

Co-authored-by: Ivan Vergés <ivan@platoniq.net>
andreslucena added a commit that referenced this pull request Jul 13, 2022
@andreslucena @microstudi
Backport 'Remove description from questions in elections' to v0.26 (#…
dc1f524 
…9522)

* Remove description from questions in elections (#9401)

* remove description

* remove description from tests

* send empty description to keep bulletin board happy

* fix schema

* fix question type spec

* remove description from questions in the DB

* remove failing spec

* Fix Rails version on migration

Co-authored-by: Ivan Vergés <ivan@platoniq.net>
eliegaboriau pushed a commit to eliegaboriau/decidim that referenced this pull request Oct 25, 2022
@microstudi @eliegaboriau
Remove description from questions in elections (decidim#9401)
e3b3893 
* remove description

* remove description from tests

* send empty description to keep bulletin board happy

* fix schema

* fix question type spec

* remove description from questions in the DB

* remove failing spec
@alecslupu alecslupu added this to the 0.27.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andreslucena andreslucena andreslucena approved these changes

@ahukkanen ahukkanen ahukkanen approved these changes

Assignees
No one assigned
Labels
contract: e-voting Barcelona City Council contract module: elections type: fix PRs that implement a fix for a bug
Projects
None yet
Milestone
0.27.0
Development

Successfully merging this pull request may close these issues.

Question description isn't used in Elections
4 participants
@microstudi @ahukkanen @andreslucena @alecslupu