Giving the administrator the option to hide polls #4819

arthur1041 · 2022-04-29T19:23:44Z

References

Related to issue #3535

Objectives

Make it possible to the administrator set a poll as published or not at its creation/update. When the poll is set published it will be shown at the polls page /polls. If the poll is not published it will not be shown.

Visual Changes

Addition of a new checkbox in the create/update poll form:

Notes

Currently the "published" attribute in the poll model is not used to filter polls. I saw that it was not set to true at any moment, so it was not being used. With the new feature brought by this PR, the "published" atribute is now being used. If a poll has the "published" attribute set to true it will be exhibited and vice versa. All the already existing polls need to have it set to true in order for them to be shown, so the following SQL script needs to be executed: update polls set published = true

config/locales/en/activerecord.yml

spec/system/admin/poll/polls_spec.rb

javierm · 2022-05-04T14:55:18Z

Hi, @arthur1041 😄. Thanks for your contribution! 🙏

If I understand it correctly, the original issue was about adding a "preview" feature to polls, similar to what we've got in budgets, so administrators could access non-published polls in the public page. Is that correct?

As far as I can see, with the changes in this pull request, administrators still can't access non-published polls in the public page 🤔. They can of course access them in the admin area, but that's something they can already do without this pull request by setting the publication date of the poll in the future.

Is there something I'm missing? Please let me know if that's the case!

Another thing to consider is that I think it shouldn't be possible to "unpublish" a poll once it's received votes, just like it isn't possible to delete a poll once it's received votes. Do you think this makes sense?

arthur1041 · 2022-05-04T18:14:59Z

Hi, @arthur1041 smile. Thanks for your contribution! pray

If I understand it correctly, the original issue was about adding a "preview" feature to polls, similar to what we've got in budgets, so administrators could access non-published polls in the public page. Is that correct?

As far as I can see, with the changes in this pull request, administrators still can't access non-published polls in the public page thinking. They can of course access them in the admin area, but that's something they can already do without this pull request by setting the publication date of the poll in the future.

Is there something I'm missing? Please let me know if that's the case!

Another thing to consider is that I think it shouldn't be possible to "unpublish" a poll once it's received votes, just like it isn't possible to delete a poll once it's received votes. Do you think this makes sense?

Yes. I read the issue again and I saw the admistrator needs to see the poll in the public page even if it's not published. I'll work on a solution for it.

Yeah, it also makes sense not to be able to unpublish a poll once it received votes, I'll add this restriction.

… on the public polls page

arthur1041 · 2022-05-18T13:35:35Z

@javierm Just pushed two new commits containing the modifications that you mentioned in your last comment.

javierm · 2022-06-02T13:25:37Z

@arthur1041 Thanks once again! 🙏

We're currently preparing version 1.5; we'll review this pull request after that. Sorry for the delay! 🙏 And thank you for your patience 😌.

javierm

@arthur1041 Sorry for the late review!

I'm also very sorry because apparently there was another team working on a similar issue and due to miscommunication we didn't know about it.

So I've left a few comments regarding this pull request but, at the same time, I don't know whether this pull request could be merged if you apply the changes 🤦.

I apologize for this whole situation 🙏.

javierm · 2022-08-02T14:32:38Z

app/controllers/polls_controller.rb

    @polls = Kaminari.paginate_array(
-      @polls.created_by_admin.not_budget.send(@current_filter).includes(:geozones).sort_for_list(current_user)
+      administrator ? @polls.created_by_admin.not_budget.send(@current_filter).includes(:geozones).sort_for_list(current_user) :
+      @polls.created_by_admin.not_budget.published.send(@current_filter).includes(:geozones).sort_for_list(current_user)


This could be tricky because the default vaule of the published column in the polls table is false. That means existing polls would no longer appear if we add this filter 🤔.

For now, feel free to ignore this scenario 👌, since there's another pull request dealing with poll restrictions (#4859) which has been developed independently, and we need to check it before deciding how to handle with this case.

javierm · 2022-08-02T14:43:46Z

app/controllers/polls_controller.rb

+    administrator = current_user ? current_user.administrator? : false
+
    @polls = Kaminari.paginate_array(
-      @polls.created_by_admin.not_budget.send(@current_filter).includes(:geozones).sort_for_list(current_user)
+      administrator ? @polls.created_by_admin.not_budget.send(@current_filter).includes(:geozones).sort_for_list(current_user) :
+      @polls.created_by_admin.not_budget.published.send(@current_filter).includes(:geozones).sort_for_list(current_user)


Maybe we could keep this line the way it was and use abilities instead 🤔. So we keep the same code here:

@polls.created_by_admin.not_budget.send(@current_filter).includes(:geozones).sort_for_list(current_user)

And in the app/models/abilities/everyone.rb file, we change:

- can :read, Poll + can :read, Poll, published: true

That way this authorization check will also be applied to the show action.

What do you think?

javierm · 2022-08-02T14:47:19Z

app/views/polls/_poll_group.html.erb

@@ -17,6 +17,10 @@
        <div class="icon-poll-answer already-answer" title="<%= t("polls.index.already_answer") %>">
          <span class="show-for-sr"><%= t("polls.index.already_answer") %></span>
        </div>
+      <% elsif current_user.administrator? && !poll.published %>


I think we don't need the first condition since unpublished polls will only be loaded for administrators 🤔.

Suggested change

<% elsif current_user.administrator? && !poll.published %>

<% elsif !poll.published %>

javierm · 2022-08-02T14:50:13Z

spec/system/budget_polls/polls_spec.rb

@@ -3,7 +3,7 @@
 describe "Polls" do
  context "Public index" do
    scenario "Budget polls should not be listed" do
-      poll = create(:poll)
+      poll = create(:poll, :published)


In order to simplify the tests, how about changing the polls factory so published is set to true by default, like we do for budgets, proposals and legislation processes? Then we can remove many of these changes and change only the tests where we're dealing with draft polls.

javierm · 2022-08-02T14:52:16Z

app/views/admin/poll/polls/_form.html.erb

+  <div class="row">
+    <div class="clear">
+      <div class="margin-top small-6 medium-6 column">
+        <%= f.check_box :published, :disabled => polls_has_votes(@poll) %>


Suggested change

<%= f.check_box :published, :disabled => polls_has_votes(@poll) %>

<%= f.check_box :published, disabled: @poll.voters.any? %>

This way we can remove the polls_has_votes helper method.

javierm · 2022-08-02T14:56:07Z

app/models/poll.rb

@@ -143,6 +144,12 @@ def date_range
    end
  end

+  def published_checkbox
+    if !published && polls_has_votes?(self)


Suggested change

if !published && polls_has_votes?(self)

if !published && voters.any?

This way we can remove the polls_has_votes? method.

javierm · 2022-08-02T16:14:36Z

app/views/admin/poll/polls/_form.html.erb

@@ -22,7 +22,7 @@
      </div>

      <div class="small-12 column">
-        <%= translations_form.text_area :summary, rows: 4 %>
+        <%= translations_form.text_field :summary, rows: 4 %>


Is this change related to this pull request? 🤔

javierm · 2023-02-21T16:03:22Z

Closing as incompatible because the changes in #4904 aren't compatible with this approach. @arthur1041 Sorry about that!

arthur1041 added 4 commits April 29, 2022 14:36

Adding a filter to show only polls that were set to "published"

dbb4c70

Allowing the "published" atribute to be persisted in the database

00bfa2c

Creating a checkbox to indicate if the poll must be published or not

4469fd3

Creation of the tests for the published polls filter

666247b

javierm added this to Reviewing in Consul Democracy via automation May 2, 2022

wenderjean reviewed May 2, 2022

View reviewed changes

config/locales/en/activerecord.yml Outdated Show resolved Hide resolved

spec/system/admin/poll/polls_spec.rb Show resolved Hide resolved

javierm assigned arthur1041 May 3, 2022

Changing label name

2ca0a8b

javierm self-assigned this May 4, 2022

javierm moved this from Reviewing to Doing in Consul Democracy May 4, 2022

arthur1041 added 2 commits May 18, 2022 10:31

Restricting moderators and voluteers from viewing non published polls…

04098f5

… on the public polls page

Preventing polls with votes from being unpublished

1ab5210

javierm moved this from Doing to Reviewing in Consul Democracy May 18, 2022

javierm reviewed Aug 2, 2022

View reviewed changes

javierm moved this from Reviewing to Doing in Consul Democracy Aug 2, 2022

javierm closed this Feb 21, 2023

javierm removed this from Doing in Consul Democracy Feb 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Giving the administrator the option to hide polls #4819

Giving the administrator the option to hide polls #4819

arthur1041 commented Apr 29, 2022

javierm commented May 4, 2022

arthur1041 commented May 4, 2022

arthur1041 commented May 18, 2022 •

edited

Loading

javierm commented Jun 2, 2022

javierm left a comment

javierm Aug 2, 2022

javierm Aug 2, 2022

javierm Aug 2, 2022

javierm Aug 2, 2022

javierm Aug 2, 2022

javierm Aug 2, 2022

javierm Aug 2, 2022

javierm commented Feb 21, 2023

	<% elsif current_user.administrator? && !poll.published %>
	<% elsif !poll.published %>

	<%= f.check_box :published, :disabled => polls_has_votes(@poll) %>
	<%= f.check_box :published, disabled: @poll.voters.any? %>

	if !published && polls_has_votes?(self)
	if !published && voters.any?

Giving the administrator the option to hide polls #4819

Giving the administrator the option to hide polls #4819

Conversation

arthur1041 commented Apr 29, 2022

References

Objectives

Visual Changes

Notes

javierm commented May 4, 2022

arthur1041 commented May 4, 2022

arthur1041 commented May 18, 2022 • edited Loading

javierm commented Jun 2, 2022

javierm left a comment

Choose a reason for hiding this comment

javierm Aug 2, 2022

Choose a reason for hiding this comment

javierm Aug 2, 2022

Choose a reason for hiding this comment

javierm Aug 2, 2022

Choose a reason for hiding this comment

javierm Aug 2, 2022

Choose a reason for hiding this comment

javierm Aug 2, 2022

Choose a reason for hiding this comment

javierm Aug 2, 2022

Choose a reason for hiding this comment

javierm Aug 2, 2022

Choose a reason for hiding this comment

javierm commented Feb 21, 2023

arthur1041 commented May 18, 2022 •

edited

Loading