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

Add offline ballot sheet votes to the total vote count #4775

Merged

Conversation

jorgebg
Copy link
Contributor

@jorgebg jorgebg commented Feb 7, 2022

References

A few days ago I was trying the offline votes upload form (ballot sheet), and noticed that the votes were not being included into the total vote count of the budget investments showed in the results page. I thought this could possible be a bug.

Objectives

Add the ballot sheet votes to the total vote count of participatory budgeting investments.

Context

Note: Please let me know if I'm missing something, I'm pretty new to Consul, and I'm a RAILS newbie as well.

If I understood properly, offline votes of participatory budgeting must be uploaded via the ballot sheet CSV data form. I tried this form on my local installation, and the ballot sheet was uploaded, but the votes were not added to the total count of the investments votes showed in the results page.

Poll::BallotSheet.verify_ballots is the method that takes care of registering the votes for a given ballot sheet. After searching the code, the only reference I found to this method was in its spec, where it was being called explicitely.

Now, I'm not aware of any RAILS API making implicit calls to this method (with my little RAILS ecosystem knowledge). If neither the app code or the 3rd party libraries are calling it, then how are these votes being counted in?

Implementation details

  • Added the Poll::BallotSheet.verify_ballots to the Active Record after_create callback, so the votes are added right after ballot sheet creation.
  • Removed the explicit call to Poll::BallotSheet.verify_ballots from the spec.

Visual Changes

None

Notes

None

@jorgebg jorgebg changed the title Add ballot sheet votes to the total count Fix: Add ballot sheet votes to the total vote count Feb 7, 2022
@jorgebg jorgebg changed the title Fix: Add ballot sheet votes to the total vote count BUGFIX: Add ballot sheet votes to the total vote count Feb 7, 2022
@jorgebg jorgebg changed the title BUGFIX: Add ballot sheet votes to the total vote count BUGFIX: Add offline ballot sheet votes to the total vote count Feb 7, 2022
@jorgebg jorgebg changed the title BUGFIX: Add offline ballot sheet votes to the total vote count BUGFIX: Offline ballot sheet votes not being added to the total vote count Feb 7, 2022
@jorgebg
Copy link
Contributor Author

jorgebg commented Feb 7, 2022

@javierm @voodoorai2000 @taitus @Senen I'm sorry for the mentions but I think this could be an important bug (I'm inexperienced so I might be wrong though, but I prefer to trigger a false alarm rather than a late alarm)

@javierm javierm added this to Reviewing in Consul Democracy via automation Feb 9, 2022
@javierm
Copy link
Member

javierm commented Feb 9, 2022

Hi, @jorgebg 😄. Thank you for the pull request! We'll have a look when we've got time (probably not this week, though).

@jorgebg
Copy link
Contributor Author

jorgebg commented Mar 4, 2022

Hi @javierm! Looks like the linter check failed. I think it's related to the way it checks out the repository, but I'm not sure. Please, can you trigger it again, just in case it was a temporary error? Thanks in advance!

@javierm
Copy link
Member

javierm commented Mar 4, 2022

Hi, @jorgebg 😄.

Yes, that's driving us crazy 😅. It tooks us about five attempts until we finally made it possible to run linters on pull requests by contributors like you, and then there was a change in Github Actions and it isn't working anymore. If you've got experience with this issue and know the solution, pull requests are welcome!

I've run the linters locally; you can do so too by running bundle exec pronto run on your machine. Here's the result:

spec/models/poll/ballot_sheet_spec.rb:5 W: Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body beginning. (https://rubystyle.guide#empty-lines-around-bodies)

@jorgebg
Copy link
Contributor Author

jorgebg commented Mar 5, 2022

Thank you, @javierm. I'm sorry, unfortunately I couldn identify the issue with Github Actions.

I removed the empty line pronto was complaining about. Not sure if pronto is working properly in my local dev, as I got no output after running it. The exit code was 0, though. If the linter still complains I'll take a look to my pronto installation.

@javierm
Copy link
Member

javierm commented Mar 6, 2022

@jorgebg Pronto displays no output locally when everything is alright 👌; it you checkout the commit having the extra blank line and execute pronto run, you should get the output I mentioned earlier.

@javierm javierm force-pushed the call_verify_ballots_after_create branch 2 times, most recently from ff8872a to 1070fb8 Compare March 21, 2022 19:16
@javierm javierm added the Bug label Mar 21, 2022
@javierm javierm changed the title BUGFIX: Offline ballot sheet votes not being added to the total vote count Add offline ballot sheet votes to the total vote count Mar 21, 2022
@javierm
Copy link
Member

javierm commented Mar 21, 2022

@jorgebg Thank you once again for the bug report and the pull request 🎉.

The changes look good to me 👍. I've updated the pull request to our latest master branch and expanded it a little bit by adding a system spec to verify the votes are taken into account when calculating the winners.

We need to test a couple more things. As soon as we verify them, we'll merge this pull request 👍.

Consul Democracy automation moved this from Reviewing to Testing Mar 21, 2022
@javierm javierm force-pushed the call_verify_ballots_after_create branch from 7e69f0a to 34335e9 Compare March 21, 2022 19:33
While writing the test, we noticed it didn't work because the labels
weren't correctly generated, so we're fixing them as well.
@javierm javierm force-pushed the call_verify_ballots_after_create branch from 34335e9 to 3f84ab0 Compare March 21, 2022 20:34
@javierm javierm self-assigned this Mar 22, 2022
@taitus taitus self-assigned this Mar 22, 2022
@javierm javierm merged commit f6c4d70 into consuldemocracy:master Mar 22, 2022
Consul Democracy automation moved this from Testing to Release 1.5.0 Mar 22, 2022
@jorgebg jorgebg deleted the call_verify_ballots_after_create branch June 13, 2022 22:42
@jorgebg
Copy link
Contributor Author

jorgebg commented Jun 13, 2022

I'm so glad this worked out! Thanks for reviewing it, @javierm, @taitus. So happy to help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants