liquidvotingio / decidim-module-liquidvoting

GNU Affero General Public License v3.0
4 stars 0 forks source link

Initial integration with Decidim proposals #27

Closed oliverbarnes closed 3 years ago

oliverbarnes commented 4 years ago

User Scenarios:

Other tasks:

oliverbarnes commented 4 years ago

Look into possible personal data privacy issues with email addresses, and how to address them

https://www.archive360.com/blog/is-data-sovereignty-a-myth-in-the-age-of-the-cloud (reference blog post, not interested in the product)

https://www.linode.com/global-infrastructure/ (location of Linode datacenters. They have one in the EU, in Frankfurt)

oliverbarnes commented 4 years ago

During our pairing/mob session tomorrow we can I think we can stop the current spike and start on a proper implementation on top of a fork of coopdevs/decidim-module-action_delegator.

Maybe we can get something going before our meeting with Coopdevs next week.

oliverbarnes commented 4 years ago

At our mob session today we:

The code here (liquidvotingio/decidim) is now stale, and we'll replace it with our 0.22.0 instance during the next session.

davefrey commented 4 years ago

At our mob today, our goal was (1) to unblock this new coopdevs-based spike work on macos, and (2) get to a running version of decidim with gems decidim-consultations and decidim-action_delegator enabled.

The macos block:

When running decidim decidim-instance on macos, the script fails like this:

/Users/davefrey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/bootsnap-1.4.4/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:22:in `require': no implicit conversion of String into Integer (TypeError)

This is a problem with bootsnap v1.4.4 and prevents any rails command from running.

Since the decidim script force-installs a Gemfile with a bootsnap spec that results in 1.4.4, the only solution we found (we tried several, including --skip-bundle) was to edit the upstream templates used to generate that Gemfile, specifying gem "bootsnap", "~> 1.4.6" in both of these files in the decidim-generators gem:

  ~/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/decidim-generators-0.22.0/
    Gemfile
    lib/decidim/generators/component_templates/Gemfile.erb

With that in place, the happy path was:

> # 1. get a basic decidim instance + database
> decidim decidim-instance
> cd decidim-instance
> bundle exec rails db:setup      # this didn't run the migrations!
> bundle exec rake db:migrate
> bundle exec rake db:seed        # this failed like this, we moved on:
  # ActiveRecord::RecordInvalid: Validation failed: Organization logo Error processing image
  # /Users/davefrey/wd/liquidvoting/decidim-instance/db/seeds.rb:9:in `<main>'

> # 2. create the System Admin user, per `decidim-instance/README.md`

> # 3. add the consultations module
> # add `gem "decidim-consultations", "0.22.0"` to `Gemfile`
> bundle
> bundle exec rails decidim_consultations:install:migrations
> bundle exec rails db:migrate

> # 4. add the action_delegator module, pointing to the local repo
> # add `gem "decidim-action_delegator", path: "../decidim-module-action_delegator"` to `Gemfile`
> bundle
> bundle exec rails decidim_action_delegator:install:migrations
> bundle exec rails db:migrate

> # 5. run the server and log in as system admin user:
> bundle exec rails server
> # go to `localhost:3000/system` and log in

We stopped there. We think this is a clean install, with the exception of the failed db:seed task, we don't know how much data is missing.

davefrey commented 4 years ago

@oliverbarnes a fix for our "no implicit conversion of String into Integer (TypeError)" problem is in the previous comment.

jinjagit commented 4 years ago

After getting decidim-instance + consultations + action-delegator running and fully seeded today, we realized a few things re. the imagined coopdevs integration spike:

Coopdevs model:

Implications:

jinjagit commented 3 years ago

This issue may well have some overlaps with #9 (Improve UX)

jinjagit commented 3 years ago

WIP - various vote-button stylings x various vote_button partial conditional states (for unmodified decidim + development_app):

if proposal.rejected? || proposal.withdrawn?
proposal_rejected else
----If component_settings.participatory_texts_enabled? && from_proposals_list proposals-p-text-enabled ----else
--------if !current_user
------------if current_settings.votes_blocked?
votes_blocked ------------else not_cur_user_not_blocked ------------end --------else ------------if @voted_proposals ? @voted_proposals.include?(proposal.id) : proposal.voted_by?(current_user) proposal_voted_by ------------else ----------------if proposal.maximum_votes_reached? && !proposal.can_accumulate_supports_beyond_threshold && current_component.participatory_space.can_participate?(current_user) support_limit_reached ----------------else --------------------if vote_limit_enabled? && remaining_votes_count_for(current_user) == 0 vote_limit_enabled --------------------elsif current_settings.votes_blocked? || !current_component.participatory_space.can_participate?(current_user) votes_blocked --------------------else not_cur_user_not_blocked --------------------end ----------------end ------------end --------end ----end end

jinjagit commented 3 years ago

Add component: proposals

Currently, we plan to avoid recording each user vote in the decidim proposals-proposals-votes table, since this would require either reproducing our api delegation(s) mechanism on the decidim side, or passing data on all votes created/deleted back to decidim whenever a create/delete vote/delegation action is executed. This means we would need to grey-out / deactivate / hide the 'Support limit per participant' and 'Minimum supports per user' in the admin view when creating the proposals component (or add these limits to our api and get the relevant values / booleans from our api when needed).

proposals_user_stuff

davefrey commented 3 years ago

Maybe this is a decent place to summarize/explain the changes I've pushed?

I've introduced a LV state object that encapsulate LV proposal queries like "how many votes". The intent is that controllers acquire this LV state object for the web request, and then views reference it for what used to be point queries to the LV api.

Decidim::Liquidvoting::Client changed a lot: I minimized the public api, and restructured the file to more easily read public / private / graphql query definitions. The public api is now just the mutations (create/delete votes and delegations), and a getter for the current LV state object.

I've left unanswered the question of whether to continue to manage the proposals.proposal_votes_count cached attribute by updating it on vote events, or abandon it completely (in both cases, LV is the system of record for vote counts).

If we take the "manage" approach, we'll need to back out some overrides, and step up to keeping the attribute in sync with LV. This has the advantage of leaving all the references to the proposal_votes_count in the Decidim codebase untouched.

If we take the "abandon" approach, we'll need to "decidim override" another 5-6 files to catch the remaining refs to proposal_votes_count, replacing them with references to our LV state object; we may want to disable/fail the attribute's getter and setter as well.

We're halfway between the two for the moment. Read/write usage of the attribute is logged as a "Liquidvoting" warning.

oliverbarnes commented 3 years ago

Thanks for the detailed update 👍 I'm reviewing the changes, will comment in the PR and we could have a call at some point to discuss the open questions

davefrey commented 3 years ago

Re: abandon-or-manage proposals.proposal_votes_count: After pairing, we agreed to take the "abandon" approach. This means we will override all of the ~20 decidim files that reference the attribute, to instead reference LV state. Also, we won't bother raising exceptions or otherwise try to flush out uses of the attribute that we missed. "Abandon without prejudice" :-)

davefrey commented 3 years ago

The checklist includes issue #18 moving voting and delegating to ajax calls; votes are working, delegation is not, I propose we remove #18 from the current issue and from PR #25, and let us track the delegation part on #18 and a new PR.

oliverbarnes commented 3 years ago

Sounds good - let's also punt adding tests (#16) to a separate PR.