Feature/2348 proposal selection from accountability with autoComplete by tramuntanal · Pull Request #2584 · decidim/decidim

tramuntanal · 2018-01-26T12:06:25Z

🎩 What? Why?

Widget to allow the administrators to associate Proposals to results in a more agile manner.

📌 Related Issues

Related to Accountability: Improve proposal selection from admin vía ID and autocompletion #2348 Accountability: Improve proposal selection from admin vía ID and autocompletion

📋 Subtasks

Add CHANGELOG entry
Refactor code
When a proposal is related from the result this proposal should automatically have the label Accepted.
NOTE: we are going to need this feature to be usable for the relation between proposals-components core team should review it.

📷 Screenshots (optional)

codecov · 2018-01-31T10:43:29Z

Codecov Report

Merging #2584 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2584      +/-   ##
==========================================
+ Coverage   98.73%   98.73%   +<.01%     
==========================================
  Files        1647     1649       +2     
  Lines       39070    39172     +102     
==========================================
+ Hits        38575    38677     +102     
  Misses        495      495

Crashillo · 2018-02-14T14:00:13Z

You may take a look at #2684. The mentions component contains the autocomplete functionality, handled by the front-end.
We should agree on how to get the data between back & front. cc/ @decidim/lot-core

tramuntanal · 2018-02-15T11:48:04Z

@Crashillo , @decidim/lot-core I'm not sure about the callbacks this new component offers but what I see in #2684 is different from what I need.

Use case 1: If text starts with # then the autocomplete search is by ID
Use case 2: Autocomplete starts by default no symbol for starting it is required, autocompletion is done by title.

@decidim/product @decidim/lot-core I also think that this issue should have been taken into account before implementing a custom autocomplete solution because it took 2 weeks of work to have the current implementation with data-picker and now we're ignoring it and:

trying to add a new autocompletion solution when here we already had a flexible one.
modifying the design (seems small changes, but many javascript work for the data-picker and poup coordination will have to be thrown away?)

Crashillo · 2018-02-15T12:03:02Z

The good point of Tribute.js is that you can initialize it whatever you want with.
We can define a set of rules for each use case, for instance, talking about hashtags, the trigger is # and the values (the source) points to an endpoint which returns an array object-like json with all the hashtags we're dealing with.
Same history, for the autocomplete, it triggers when a user types something, and points to a json with the data required to fill out the input.

tramuntanal · 2018-02-15T12:25:18Z

Yes, the same characteristics as the ultra fast autoComplete 👅

Also we have a collision with symbol '#' wich will have different use cases. I think it is better to stay with autoComplete for this issue.

…ls'.

…ntability-autoComplete

tramuntanal · 2018-02-26T18:39:03Z

Hi @decidim/lot-core, how can I make jquery-autocomplete.css and .js files being ignored by codeclimate? I'm adding it into codeclimate.yml excludes but keeps analyzing them.

mrcasals · 2018-02-27T08:24:16Z

@tramuntanal please rebase against master, move those files to the vendor folder and remove the changes in the .codeclimate.yml file, I think that should fix it.

… to a Result.

…oComplete' of https://github.com/decidim/decidim into feature/2348-proposal_selection_from_accountability-autoComplete

…ntability-autoComplete

tramuntanal · 2018-03-01T16:09:04Z

@decidim/lot-core all checks pass now 😄

…ry parameter from :q to :term.

josepjaume

Please just remove these comment lines and that's it!

josepjaume · 2018-03-02T12:58:20Z

decidim-accountability/app/controllers/decidim/accountability/admin/results_controller.rb

+          end
+        end
+
+        #-----------------------------------------------------------------------


This is now not needed since there's nothing in the middle haha

josepjaume · 2018-03-02T12:58:35Z

decidim-accountability/app/controllers/decidim/accountability/admin/results_controller.rb

+
        private

+        #-----------------------------------------------------------------------


Now there's no need to have this here 👍

mrcasals · 2018-03-02T13:35:47Z

decidim-accountability/app/forms/decidim/accountability/admin/result_form.rb

        def proposals
-          @proposals ||= Decidim.find_resource_manifest(:proposals).try(:resource_scope, current_feature)&.order(title: :asc)&.pluck(:title, :id)
+          @proposals ||= Decidim.find_resource_manifest(:proposals)
+                                .try(:resource_scope, current_feature)


Fix identation 😄

I think it's a bug that rubocop didn't catch this. I reported it: rubocop/rubocop#5621...

mrcasals · 2018-03-02T13:35:49Z

...countability/app/assets/javascripts/decidim/accountability/admin/accountability_admin.js.es6

+      },
+      renderItem: function (item, search) {
+        let sanitizedSearch= search.replace(/[-/\\^$*+?.()|[\]{}]/g, '\\$&');
+        let re= new RegExp(`(${sanitizedSearch.split(' ').join('|')})`, "gi");


Please add spaces between the variable name and = 😄

mrcasals · 2018-03-02T13:36:12Z

decidim-accountability/app/views/decidim/accountability/admin/results/_form.html.erb

-                        {},
-                        { multiple: true, class: "chosen-select" }
-        %>
+        <% picker_options= { id: "decidim_accountability_proposals", class: "picker-multiple", name: 'result[proposal_ids]', multiple: true }


Space between variable name and = please 😄

mrcasals · 2018-03-02T13:38:39Z

decidim-core/lib/decidim/form_builder.rb

+    #
+    # Returns an html String.
+    def data_picker(attribute, options = {}, prompt_params = {})
+      picker_options = { id: "#{@object_name}_#{attribute}", class: "picker-#{options[:multiple] ? "multiple" : "single"}",


Can you normalize this hash please? Each key in a single line, etc

Note that rubocop explicitly accepts this style and this file is using that same style in other places, so I think it's fine to leave it since otherwise we're introducing "in-file" inconsistency.

As a note, I made a very related feature request to rubocop a while ago, but nobody has stepped to implement it. It's not even clear if it should be a different cop or a configuration option for existing cops.

josepjaume · 2018-03-05T08:35:18Z

decidim-accountability/app/controllers/decidim/accountability/admin/results_controller.rb

+          end
+        end
+
+        #-----------------------------------------------------------------------


@tramuntanal remember to remove this :)

josepjaume · 2018-03-05T08:35:34Z

decidim-accountability/app/controllers/decidim/accountability/admin/results_controller.rb

+
        private

+        #-----------------------------------------------------------------------


Aaand this :)

tramuntanal · 2018-03-05T09:48:37Z

This PR is ready again @decidim/lot-core

mrcasals · 2018-03-05T11:51:58Z

@tramuntanal I still see some comments that have not been addressed yet, can you remove those extra lines please?

josepjaume · 2018-03-05T15:59:01Z

@tramuntanal - @mrcasals is referring to the comment lines (#------) and the identation issues. Once that's solved, we can merge this! We'll release 0.10-pre tomorrow morning and it'd be nice to have this!

tramuntanal · 2018-03-05T17:50:42Z

@decidim/lot-core I've removed both better readability lines on your command, feeling bad about that.

josepjaume · 2018-03-05T19:20:17Z

@tramuntanal hey no prob! There's a lot of things going on right now, only natural to forget about some comments now and then 👍

Delegated approval to me via telegram xD

josepjaume · 2018-03-05T19:23:36Z

Merging!! 🎉 🎉 🎉 🎉 🎉 🎉 🎉

tramuntanal · 2018-03-06T08:18:35Z

@josepjaume I was sad for having to remove the lines that I think were positive because they added better readability to code but well..

Merged at the end 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉

ghost assigned tramuntanal Jan 26, 2018

ghost added the in-progress label Jan 26, 2018

oliver and others added 10 commits February 26, 2018 11:27

[DOC] Some Data-picker documentation.

f5b5558

[WIP] Check how data-picker works.

07f6833

Update data-picker.md

e2adb89

[DOC|WIP] DataPicker documentation.

2b5b250

[WIP] first approach reached, but needs to be reworked.

1957c99

[WIP] try ajax.

0aaa498

[WIP] First component proposal.

cd9617a

[REFACTOR|FIX] Remove legacy require for now unused 'jquery-atwho-rai…

2896a11

…ls'.

[CONFIG] Remove reminiscenses of discarded gem.

a9a4120

[REFACTOR] Encapsulate DataPicker component into form builder.

4f98de6

tramuntanal force-pushed the feature/2348-proposal_selection_from_accountability-autoComplete branch from 50fbadd to 4f98de6 Compare February 26, 2018 10:28

isaacmg410 changed the title ~~[WIP] Feature/2348 proposal selection from accountability with autoComplete~~ Feature/2348 proposal selection from accountability with autoComplete Feb 26, 2018

tramuntanal and others added 6 commits February 26, 2018 17:08

Merge branch 'master' into feature/2348-proposal_selection_from_accou…

305dfb2

…ntability-autoComplete

[TEST] Adjust ResultForm specs to new needs.

42554e1

Added CHANGELOG.md entry.

3058152

Apply Rubocop's corrections.

e17982b

Make Codeclimate to ignore autoComplete library dependency.

c10f0d2

Apply rubocop corrections to decidm-core.

b6f8f67

Oliver Valls added 3 commits February 27, 2018 12:17

[FEATURE] Make Proposals being automatically accepted when associated…

be6eda1

… to a Result.

Merge branch 'feature/2348-proposal_selection_from_accountability-aut…

5ad4930

…oComplete' of https://github.com/decidim/decidim into feature/2348-proposal_selection_from_accountability-autoComplete

Merge branch 'master' into feature/2348-proposal_selection_from_accou…

f62dccd

…ntability-autoComplete

Oliver Valls and others added 3 commits March 1, 2018 15:51

[TEST|FIX] Make test pass.

796ac74

[REFACTOR] Apply js linter fixes.

9a54697

[REFACTOR] Apply eslint corrections.

0b4bb4c

ghost assigned agustibr Mar 1, 2018

[REFACTOR] Reenable linter in a js block. This required to rename que…

aee87b8

…ry parameter from :q to :term.

josepjaume suggested changes Mar 2, 2018

View reviewed changes

mrcasals reviewed Mar 2, 2018

View reviewed changes

deivid-rodriguez mentioned this pull request Mar 2, 2018

Enable space-infix-ops eslint cop #2889

Merged

Oliver Valls added 4 commits March 2, 2018 17:17

Vertical align chain of method calls.

8070af2

Add spaces between the variable name and = in accountability_admin.js.

9e28c8c

Add spaces between the variable name and =.

045d192

"Normalize" hash: Each key in a single line, etc.

c38c29a

josepjaume suggested changes Mar 5, 2018

View reviewed changes

josepjaume added this to the Release v.0.10.0 milestone Mar 5, 2018

Remove better readability by the rule of core.

386412e

josepjaume approved these changes Mar 5, 2018

View reviewed changes

josepjaume merged commit 5e5377b into master Mar 5, 2018

ghost removed the in-review label Mar 5, 2018

josepjaume deleted the feature/2348-proposal_selection_from_accountability-autoComplete branch March 5, 2018 19:23

josepjaume mentioned this pull request Mar 5, 2018

Accountability: Improve proposal selection from admin vía ID and autocompletion #2348

Closed

4 tasks

Xfolchf mentioned this pull request Apr 19, 2018

Linking proposals to results for accountability #3167

Closed


		private

		#-----------------------------------------------------------------------

Uh oh!

Conversation

tramuntanal commented Jan 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎩 What? Why?

📌 Related Issues

📋 Subtasks

📷 Screenshots (optional)

Uh oh!

codecov bot commented Jan 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Crashillo commented Feb 14, 2018

Uh oh!

tramuntanal commented Feb 15, 2018

Uh oh!

Crashillo commented Feb 15, 2018

Uh oh!

tramuntanal commented Feb 15, 2018

Uh oh!

tramuntanal commented Feb 26, 2018

Uh oh!

mrcasals commented Feb 27, 2018

Uh oh!

tramuntanal commented Mar 1, 2018

Uh oh!

josepjaume left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrcasals Mar 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tramuntanal commented Mar 5, 2018

Uh oh!

mrcasals commented Mar 5, 2018

Uh oh!

josepjaume commented Mar 5, 2018

Uh oh!

tramuntanal commented Mar 5, 2018

Uh oh!

josepjaume commented Mar 5, 2018

Uh oh!

josepjaume commented Mar 5, 2018

Uh oh!

tramuntanal commented Mar 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tramuntanal commented Jan 26, 2018 •

edited

Loading

codecov bot commented Jan 31, 2018 •

edited

Loading

mrcasals Mar 2, 2018 •

edited

Loading