Delegations frontoffice prototype #15

sauloperez · 2020-09-10T09:42:14Z

This implements user-facing delegations so that they can use what the admins set up using #6.

TODO

app/views/decidim/action_delegator/_delegations_modal.html.erb

Your app needs to then require this asset in application.js as follows ``` //= require decidim/action_delegator ```

This way we don't duplicate the query and we can refactor both calls at the same time. Now we perform a `SELECT 1` instead of a `SELECT *` followed by a loop over the resulting array.

In the consultations index page the consultation card's background image makes it pretty hard to read the link. White text makes things a bit better but the admin will still have to choose a rather dark image.

Apart from copying the large if/else block from `question/_vote_button.html.erb` some bits need to be changed from the `current_user` to `delegation.granter`. And things turn out not the be simple. `#allowed_to?` doesn't make it possible to only pass in a different user. We are forced to pass in the `chain` argument as well, in which we just copying the result we got from `#permission_class_chain`. Switching this method's signature to use keyboard arguments or a hash of options would solve this issue.

It mimics the regular vote button but just a bit smaller.

Regardless of their action: vote or unvote.

It works great with some images but when there is none, it becomes invisible. We assume most coops won't upload any background image.

We don't want to fix it with the actual vote button either.

We need it to know which specific vote needs to be destroyed, although Decidim destroys all author's votes anyway, but that's another topic.

sauloperez · 2020-09-16T15:05:30Z

app/views/decidim/action_delegator/_delegations_modal.html.erb

+
+          </div>
+        </div>
+      <% end %>


this is the bit that worries me the most. I copied it but then added stuff for delegations on top, so we diverged a bit.

microstudi

Why putting assets in vendor? aren't these assets part of the module?

And, I think we should include it in the view that are using it using <%= stylesheet_link_tag "decidim/action_delegator/delegations*" media: "all" %> style

sauloperez · 2020-09-17T15:52:57Z

The only easy way I found to address Close the delegations modal and re-render to reflect new state is to follow Decidim's approach on the regular vote button. Send an AJAX text/javascript request to a new endpoint that closes the delegations modal and rerenders it as done in https://github.com/decidim/decidim/blob/develop/decidim-consultations/app/views/decidim/consultations/question_votes/update_vote_button.js.erb.

We need it to know which specific vote needs to be destroyed, although Decidim destroys all author's votes anyway, but that's another topic.

Monkeypatches

I forgot we're in a loop when I first added this elements. As a result, either the browser complains or the button doesn't work, or both.

This fixes the issue where right after voting, opening the delegations modal again and the closing it, the former modal was there. I'm assuming this is because without closing and removing it the fact that has a listener attached, makes it stay in the DOM and so when we render the new one that one is still there. Or something alike. What I fail to understand is why does the reveal modal end up at the bottom of the DOM even though we render it within the div "question-<question_id>-vote-button", which holds the contents that we re-render through update_vote_button.js.erb. Something that Foundation does, I guess.

Code Climate is complaining about it

The way to tell Ruby we need to evaluate `decidim-consultations` first is through a require, which is much more reliable than the order in which gems are listed in the Gemfile. Besides, Rubocop wants them to be sorted alphabetically.

By copying the original spec file we make it easier to replace the `class_eval` with a more reliable way to extend the commands behaviour.

sauloperez · 2020-09-21T11:25:04Z

I summarized the design we agreed on at #19. Surely, other more specific issues will come out of it.

microstudi

🚀 but we should note that we need to improve some cumbersome checks in the views, change them for helpers

microstudi · 2020-09-21T13:17:46Z

app/views/decidim/action_delegator/_callout.html.erb

@@ -0,0 +1,5 @@
+<% if Decidim::ActionDelegator::Delegation.granted_to?(current_user, consultation) %>


We could probably add a helper here

yeah, probably a helper like #granted_to? makes sense

microstudi · 2020-09-21T13:18:43Z

app/views/decidim/action_delegator/_delegations_modal.html.erb

+  <div class="card card--secondary">
+    <div class="card__content">
+
+      <% Decidim::ActionDelegator::ConsultationDelegations.for(question.consultation, current_user).each do |delegation| %>


microstudi · 2020-09-21T13:19:08Z

app/views/decidim/action_delegator/_delegations_modal.html.erb

+                </div>
+              <% end %>
+            <% elsif signed_in? && question.consultation.active? %>
+              <% if allowed_to? :unvote, :question, { question: question }, [Decidim::Consultations::Permissions, Decidim::Admin::Permissions, Decidim::Permissions], delegation.granter %>


I'm not so sure about this one. IMO the mess with the permission chain should be solved by providing a new permission class of our own or similar so that we don't need to pass them here. As a result, this wouldn't diverge much from Decidim's calls to allowed_to?.

true! in any case, it would be simplified!

tracked in #24.

microstudi · 2020-09-21T13:19:22Z

app/views/decidim/action_delegator/_delegations_modal.html.erb

+                    <%= t("questions.vote_button.already_voted", scope: "decidim") %>
+                  </div>
+                <% end %>
+              <% elsif allowed_to? :vote, :question, { question: question }, [Decidim::Consultations::Permissions, Decidim::Admin::Permissions, Decidim::Permissions], delegation.granter %>


microstudi · 2020-09-21T13:21:21Z

lib/decidim/action_delegator/engine.rb

@@ -2,6 +2,7 @@

 require "rails"
 require "decidim/core"
+require "decidim/consultations"


sauloperez · 2020-09-21T13:50:22Z

Thanks for the feedback @microstudi . I created #21 to keep track of it.

Picks the latest changes from https://github.com/coopdevs/decidim-module-action_delegator/. Namely, openpoke/decidim-module-action_delegator#15.

First attempt at delegation UI

4b642d4

sauloperez self-assigned this Sep 10, 2020

sauloperez mentioned this pull request Sep 10, 2020

Delegations frontoffice prototype coopdevs/decidim#1

Closed

Provide dependencies for the warning callout

826c8ad

sauloperez commented Sep 14, 2020

View reviewed changes

app/views/decidim/action_delegator/_delegations_modal.html.erb Outdated Show resolved Hide resolved

sauloperez added 5 commits September 14, 2020 12:24

Don't show callout when no delegation is selected

56f822a

Move delegation's JS into module

75fe567

Your app needs to then require this asset in application.js as follows ``` //= require decidim/action_delegator ```

Extract delegation links as partials

b51d677

I18n-ize copies

2a910b9

Don't show the delegations link when there is none

ccb5622

sauloperez force-pushed the delegations-frontoffice branch from 0fcb17a to 4200f1a Compare September 15, 2020 07:57

sauloperez added 5 commits September 15, 2020 11:47

DRY and refactor eligible delegations check

b0ee4d4

This way we don't duplicate the query and we can refactor both calls at the same time. Now we perform a `SELECT 1` instead of a `SELECT *` followed by a loop over the resulting array.

Render delegations link in white

6f89c82

In the consultations index page the consultation card's background image makes it pretty hard to read the link. White text makes things a bit better but the admin will still have to choose a rather dark image.

Fix delegation vote button styling

395f030

It mimics the regular vote button but just a bit smaller.

Leave some margin below each vote button

ecc5cbb

Regardless of their action: vote or unvote.

sauloperez force-pushed the delegations-frontoffice branch from 4200f1a to ecc5cbb Compare September 15, 2020 09:47

sauloperez added 3 commits September 16, 2020 08:46

Render delegations link in the original color

a43d45f

It works great with some images but when there is none, it becomes invisible. We assume most coops won't upload any background image.

Make id unique

58f3aeb

We don't want to fix it with the actual vote button either.

Pass the delegation id to unvote

ec70a71

We need it to know which specific vote needs to be destroyed, although Decidim destroys all author's votes anyway, but that's another topic.

sauloperez commented Sep 16, 2020

View reviewed changes

microstudi reviewed Sep 17, 2020

View reviewed changes

override consultations views for delegation. add MD5 tests

bba22a8

sauloperez force-pushed the delegations-frontoffice branch 2 times, most recently from 16a1de0 to 6ed5fa0 Compare September 17, 2020 15:49

microstudi added 4 commits September 17, 2020 18:02

provide a generic way to monkeypatch controllers

a35ce96

set monkeypatches overrides

3b91686

make vote delegation work with overrides

b59ef0b

fix javascript, add md5 checks

a96336c

normalize locales

1584489

sauloperez force-pushed the delegations-frontoffice branch from 6ed5fa0 to 9dd20a9 Compare September 18, 2020 08:46

sauloperez and others added 2 commits September 18, 2020 16:05

Pass the delegation id to unvote

92f5f33

We need it to know which specific vote needs to be destroyed, although Decidim destroys all author's votes anyway, but that's another topic.

set correct order in gemfile

05205bf

sauloperez force-pushed the delegations-frontoffice branch from 9dd20a9 to 92f5f33 Compare September 18, 2020 14:07

sauloperez added 6 commits September 18, 2020 16:09

Merge branch 'delegations-frontoffice' into monkeypatches

3bdfbf8

Merge pull request #18 from coopdevs/monkeypatches

96e55f7

Monkeypatches

Make ids unique or rely in classes instead

40d23ee

I forgot we're in a loop when I first added this elements. As a result, either the browser complains or the button doesn't work, or both.

Remove trailing whitespace

68a9dc5

Code Climate is complaining about it

Add system spec to test delegation vote

0be0503

sauloperez force-pushed the delegations-frontoffice branch from 5947dbb to 0be0503 Compare September 21, 2020 09:32

sauloperez added 6 commits September 21, 2020 11:45

Add spec to test delegation unvote

3fc17a3

Remove no longer used vendor assets

be69d41

Test Delegation.granted_to?

916c1df

Test ConsultationDelegations query

9f89aa2

Fix and ensure we don't depend on gems' order

61d866b

The way to tell Ruby we need to evaluate `decidim-consultations` first is through a require, which is much more reliable than the order in which gems are listed in the Gemfile. Besides, Rubocop wants them to be sorted alphabetically.

Test overrides to VoteQuestion command

c106c3e

By copying the original spec file we make it easier to replace the `class_eval` with a more reliable way to extend the commands behaviour.

sauloperez mentioned this pull request Sep 21, 2020

Replace class_evals with more reliable mechanisms #19

Closed

sauloperez marked this pull request as ready for review September 21, 2020 11:25

microstudi approved these changes Sep 21, 2020

View reviewed changes

sauloperez mentioned this pull request Sep 21, 2020

Replace cumbersome checks in views with helpers #21

Open

sauloperez merged commit d7a48a5 into master Sep 21, 2020

sauloperez deleted the delegations-frontoffice branch September 21, 2020 13:53

This was referenced Sep 21, 2020

Update to latest commit CoopCat-Confederacio-de-Cooperatives/decidim-coopcat#24

Merged

Simplify calls to allowed_to? #24

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delegations frontoffice prototype #15

Delegations frontoffice prototype #15

sauloperez commented Sep 10, 2020 •

edited

Loading

sauloperez Sep 16, 2020

microstudi left a comment

sauloperez commented Sep 17, 2020 •

edited

Loading

sauloperez commented Sep 21, 2020

microstudi left a comment

microstudi Sep 21, 2020

sauloperez Sep 21, 2020

microstudi Sep 21, 2020

microstudi Sep 21, 2020

sauloperez Sep 21, 2020

microstudi Sep 21, 2020

sauloperez Sep 22, 2020

microstudi Sep 21, 2020

microstudi Sep 21, 2020

sauloperez commented Sep 21, 2020

		@@ -0,0 +1,5 @@
		<% if Decidim::ActionDelegator::Delegation.granted_to?(current_user, consultation) %>

Delegations frontoffice prototype #15

Delegations frontoffice prototype #15

Conversation

sauloperez commented Sep 10, 2020 • edited Loading

TODO

Choose a reason for hiding this comment

microstudi left a comment

Choose a reason for hiding this comment

sauloperez commented Sep 17, 2020 • edited Loading

sauloperez commented Sep 21, 2020

microstudi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sauloperez commented Sep 21, 2020

sauloperez commented Sep 10, 2020 •

edited

Loading

sauloperez commented Sep 17, 2020 •

edited

Loading