Allow associating subscopes of any level when creating new resources (decidim#1746)

leio10 · josepjaume · commit 8c05fa47d903 · 2017-08-16T10:56:39.000+02:00
* Renamed Select2Filter to Select2Field to use it outside filter forms. * Select2 appearance adapted for frontend forms. * Fixed behavior on search scopes AJAX call when passing false to include_root param. * Using select2 for scope selection on proposals new form. * Using select2 for scope selection on features admin forms. * Fixed scope retrieval on scoped resources forms. Before decidim#1500, only processes with global scope (nil value) could have features with a scope. Now this is not true, so the scope retrieval should only use the process scope when there is no scope for the resource. * Scopes controller tests improved. * Remove unused code. * Rubocop offenses. * ESLint offense * ESLint offense * Select2 capybara tests fixed. * Ignore missing global scopetranslations in I18n tests on budgets, meetings and results engines. * Simplified logic on scopes search controller. * Defined matcher for scopes controller tests to improve readability. * Defined matcher for scopes controller tests to improve readability. * Temporary usability improvement on scopes management. * Scopes search controller tests improved.
diff --git a/decidim-admin/app/views/decidim/admin/scopes/index.html.erb b/decidim-admin/app/views/decidim/admin/scopes/index.html.erb
@@ -29,6 +29,7 @@
                 <%= scope.scope_type ? translated_attribute(scope.scope_type.name) : "-" %>
               </td>
               <td class="table-list__actions">
+                <%= icon_link_to "zoom-in", scope_scopes_path(scope), t("actions.browse", scope: "decidim.admin"), class: "action-icon--browse", method: :get, data: {} %>
 
                 <% if can? :update, scope %>
                   <%= icon_link_to "pencil", ['edit', scope], t("actions.edit", scope: "decidim.admin"), class: "action-icon--edit", method: :get, data: {} %>
diff --git a/decidim-admin/config/locales/en.yml b/decidim-admin/config/locales/en.yml
@@ -123,6 +123,7 @@ en:
       actions:
         activate: Activate
         add: Add
+        browse: Browse
         configure: Configure
         confirm_destroy: Are you sure you want to delete this?
         destroy: Destroy
diff --git a/decidim-budgets/app/forms/decidim/budgets/admin/project_form.rb b/decidim-budgets/app/forms/decidim/budgets/admin/project_form.rb
@@ -40,7 +40,7 @@ def proposals
         end
 
         def scope
-          @scope ||= process_scope || current_organization.scopes.where(id: decidim_scope_id).first
+          @scope ||= current_organization.scopes.where(id: decidim_scope_id).first || process_scope
         end
 
         def category
diff --git a/decidim-budgets/app/views/decidim/budgets/admin/projects/_form.html.erb b/decidim-budgets/app/views/decidim/budgets/admin/projects/_form.html.erb
@@ -18,7 +18,7 @@
 
     <% if current_participatory_process.has_subscopes? %>
       <div class="row column" >
-        <%= form.collection_select :decidim_scope_id, subscopes_for(current_participatory_process), :id, :name %>
+        <%= form.scopes_select :decidim_scope_id, prompt: I18n.t("decidim.scopes.global"), remote_path: decidim.scopes_search_path(root: current_participatory_process.scope) %>
       </div>
     <% end %>
 
diff --git a/decidim-budgets/config/i18n-tasks.yml b/decidim-budgets/config/i18n-tasks.yml
@@ -6,6 +6,7 @@ ignore_unused:
 - "decidim.features.budgets.actions.*"
 - "activemodel.attributes.project.*"
 - "decidim.resource_links.*"
-
+ignore_missing:
+  - decidim.scopes.global
 search:
   strict: false
diff --git a/decidim-budgets/spec/shared/manage_projects_examples.rb b/decidim-budgets/spec/shared/manage_projects_examples.rb
@@ -82,7 +82,7 @@
       )
       fill_in :project_budget, with: 22_000_000
 
-      select translated(scope.name), from: :project_decidim_scope_id
+      select2 translated(scope.name), xpath: '//select[@id="project_decidim_scope_id"]/..', search: true
       select translated(category.name), from: :project_decidim_category_id
 
       find("*[type=submit]").click
diff --git a/decidim-core/app/assets/javascripts/decidim.js.es6 b/decidim-core/app/assets/javascripts/decidim.js.es6
@@ -12,6 +12,7 @@
 // = require decidim/user_registrations
 // = require decidim/account_form
 // = require decidim/select2
+// = require decidim/select2.field
 
 /* globals svg4everybody */
 
diff --git a/decidim-core/app/assets/javascripts/decidim/form_filter.component.js.es6 b/decidim-core/app/assets/javascripts/decidim/form_filter.component.js.es6
@@ -1,7 +1,5 @@
 /* eslint-disable no-div-regex, no-useless-escape, no-param-reassign, id-length */
 
-// = require ./select2.filter
-
 /**
  * A plain Javascript component that handles the form filter.
  * @class
@@ -53,7 +51,7 @@
         this.mounted = true;
         let select2Filters = this.select2Filters;
         this.$form.find('select.select2').each(function(index, select) {
-          select2Filters.push(new window.Decidim.Select2Filter(select));
+          select2Filters.push(new window.Decidim.Select2Field(select));
         });
         this.$form.on('change', 'input:not(.select2-search__field), select', this._onFormChange);
 
diff --git a/decidim-core/app/assets/javascripts/decidim/form_filter.component.test.js b/decidim-core/app/assets/javascripts/decidim/form_filter.component.test.js
@@ -4,7 +4,7 @@ window.$ = require('jquery');
 require('select2');
 
 require('./history.js.es6');
-require('./select2.filter.js.es6');
+require('./select2.field.js.es6');
 require('./form_filter.component.js.es6');
 
 const { Decidim: { FormFilterComponent } } = window;
diff --git a/decidim-core/app/assets/javascripts/decidim/select2.field.js.es6 b/decidim-core/app/assets/javascripts/decidim/select2.field.js.es6
@@ -3,11 +3,12 @@
  */
 
 ((exports) => {
-  class Select2Filter {
+  class Select2Field {
     constructor(element) {
       const selectedLang = $("html").attr('lang') || 'en';
 
       let $element = $(element);
+
       let options = {
         language: selectedLang,
         multiple: $element.attr("multiple")==="multiple",
@@ -42,5 +43,5 @@
   }
 
   exports.Decidim = exports.Decidim || {};
-  exports.Decidim.Select2Filter = Select2Filter;
+  exports.Decidim.Select2Field = Select2Field;
 })(window);
diff --git a/decidim-core/app/assets/stylesheets/decidim/plugins/_select2.scss b/decidim-core/app/assets/stylesheets/decidim/plugins/_select2.scss
@@ -23,12 +23,14 @@
       line-height: 46px;
       padding-right: 23px;
       font-weight: normal;
+      color: #1a181d;
     }
     .select2-selection__arrow {
       height: 54px;
       right: 5px;
       b {
         border-width: 6px 5px 0 5px;
+        border-top-color: #121014;
       }
     }
   }
@@ -57,4 +59,5 @@
 }
 .select2-container--default.select2-container--open .select2-selection--single .select2-selection__arrow b {
   border-width: 0 5px 6px 5px;
+  border-bottom-color: #121014;
 }
diff --git a/decidim-core/app/controllers/decidim/scopes_controller.rb b/decidim-core/app/controllers/decidim/scopes_controller.rb
@@ -13,7 +13,7 @@ def search
                else
                  current_organization.top_scopes
                end
-      root_option = if params[:include_root] && !params[:term].present?
+      root_option = if params[:include_root] == "true" && !params[:term].present?
                       if root
                         [{ id: root.id.to_s, text: root.name[I18n.locale.to_s] }]
                       else
diff --git a/decidim-core/app/helpers/decidim/scopes_helper.rb b/decidim-core/app/helpers/decidim/scopes_helper.rb
@@ -5,21 +5,6 @@ module Decidim
   module ScopesHelper
     Option = Struct.new(:id, :name)
 
-    # Public: Returns a collection of the given "scopable object" scopes,
-    # prepending a global scope. The global scope is at the beginning because
-    # it's easier to be found there. Use this helper in places like `select`
-    # elements in forms.
-    #
-    # scopable - an object including the Decidim::Scopable concern.
-    #
-    # Returns an Array.
-    def subscopes_for(scopable)
-      [Option.new("", I18n.t("decidim.scopes.global"))] +
-        scopable.subscopes.map do |scope|
-          Option.new(scope.id, translated_attribute(scope.name))
-        end
-    end
-
     # Check whether the resource has a visible scope or not.
     #
     # Returns boolean.
diff --git a/decidim-core/spec/controllers/scopes_controller_spec.rb b/decidim-core/spec/controllers/scopes_controller_spec.rb
@@ -14,12 +14,23 @@ module Decidim
 
     let(:user) { create(:user, :admin, :confirmed, organization: organization) }
     let(:query) { "" }
+    let(:params) { { term: query } }
     let(:results) { JSON.parse(response.body) }
 
+    subject { results["results"] }
+
     before do
       @request.env["decidim.current_organization"] = organization
       sign_in user, scope: :user
-      get :search, format: :json, params: { term: query }
+      get :search, format: :json, params: params
+    end
+
+    matcher :have_scopes do |expected|
+      match do |results|
+        result_texts = results.map { |r| r["text"] }
+
+        RSpec::Matchers::BuiltIn::ContainExactly.new(result_texts).matches?(expected)
+      end
     end
 
     context "basic search works" do
@@ -28,36 +39,59 @@ module Decidim
       end
 
       it "result has id" do
-        expect(results["results"].first).to have_key("id")
+        expect(subject.first).to have_key("id")
       end
 
       it "result has text" do
-        expect(results["results"].first).to have_key("text")
+        expect(subject.first).to have_key("text")
       end
     end
 
     context "search top scopes" do
-      it { expect(results["results"].count).to eq(3) }
+      it { is_expected.to have_scopes %w(Aaaa Aabb Bbbb) }
     end
 
     context "find one result" do
       let(:query) { "Bb" }
-      it { expect(results["results"].count).to eq(1) }
+      it { is_expected.to have_scopes %w(Bbbb) }
     end
 
     context "find several results" do
       let(:query) { "Aa" }
-      it { expect(results["results"].count).to eq(2) }
+      it { is_expected.to have_scopes %w(Aaaa Aabb) }
     end
 
     context "find subscopes" do
       let(:query) { "Cc" }
-      it { expect(results["results"].count).to eq(1) }
+      it { is_expected.to have_scopes %w(Cccc) }
     end
 
     context "don't find results" do
       let(:query) { "Dd" }
-      it { expect(results["results"].count).to eq(0) }
+      it { is_expected.to be_empty }
+    end
+
+    context "with root filter" do
+      let(:params) { { term: query, root: scopes.first } }
+
+      context "search top scopes" do
+        it { is_expected.to have_scopes %w(Cccc) }
+      end
+
+      context "find one result" do
+        let(:query) { "Cc" }
+        it { is_expected.to have_scopes %w(Cccc) }
+      end
+
+      context "don't find results outside the root scope" do
+        let(:query) { "Bb" }
+        it { is_expected.to be_empty }
+      end
+
+      context "include root" do
+        let(:params) { { term: query, root: scopes.first, include_root: true } }
+        it { is_expected.to have_scopes %w(Aaaa Cccc) }
+      end
     end
   end
 end
diff --git a/decidim-meetings/app/forms/decidim/meetings/admin/meeting_form.rb b/decidim-meetings/app/forms/decidim/meetings/admin/meeting_form.rb
@@ -45,7 +45,7 @@ def process_scope
 
         def scope
           return unless current_feature
-          @scope ||= process_scope || current_feature.scopes.where(id: decidim_scope_id).first
+          @scope ||= current_feature.scopes.where(id: decidim_scope_id).first || process_scope
         end
 
         def category
diff --git a/decidim-meetings/app/views/decidim/meetings/admin/meetings/_form.html.erb b/decidim-meetings/app/views/decidim/meetings/admin/meetings/_form.html.erb
@@ -36,7 +36,7 @@
     <div class="row" >
       <% if current_participatory_process.has_subscopes? %>
         <div class="columns xlarge-6" >
-          <%= form.collection_select :decidim_scope_id, subscopes_for(current_participatory_process), :id, :name %>
+          <%= form.scopes_select :decidim_scope_id, prompt: I18n.t("decidim.scopes.global"), remote_path: decidim.scopes_search_path(root: current_participatory_process.scope) %>
         </div>
       <% end %>
 
diff --git a/decidim-meetings/config/i18n-tasks.yml b/decidim-meetings/config/i18n-tasks.yml
@@ -4,5 +4,7 @@ ignore_unused:
 - "decidim.features.meetings.settings.*"
 - "activemodel.attributes.*"
 - "decidim.resource_links.*"
+ignore_missing:
+  - decidim.scopes.global
 search:
   strict: false
diff --git a/decidim-meetings/spec/shared/manage_meetings_examples.rb b/decidim-meetings/spec/shared/manage_meetings_examples.rb
@@ -96,7 +96,7 @@
     page.find(".datepicker-dropdown .hour", text: "12:00").click
     page.find(".datepicker-dropdown .minute", text: "12:50").click
 
-    select translated(scope.name), from: :meeting_decidim_scope_id
+    select2 translated(scope.name), xpath: '//select[@id="meeting_decidim_scope_id"]/..', search: true
     select translated(category.name), from: :meeting_decidim_category_id
 
     within ".new_meeting" do
@@ -209,7 +209,7 @@
       page.find(".datepicker-dropdown .hour", text: "12:00").click
       page.find(".datepicker-dropdown .minute", text: "12:50").click
 
-      select translated(scope.name), from: :meeting_decidim_scope_id
+      select2 translated(scope.name), xpath: '//select[@id="meeting_decidim_scope_id"]/..', search: true
       select translated(category.name), from: :meeting_decidim_category_id
 
       within ".new_meeting" do
diff --git a/decidim-proposals/app/assets/javascripts/decidim/proposals/add_proposal.js.es6 b/decidim-proposals/app/assets/javascripts/decidim/proposals/add_proposal.js.es6
@@ -16,6 +16,8 @@ $(() => {
       toggleInput();
       $checkbox.on('change', toggleInput);
     }
+
+    new window.Decidim.Select2Field($('#proposal_scope_id')); // eslint-disable-line no-new
   };
 
   window.DecidimProposals.bindProposalAddress();
diff --git a/decidim-proposals/app/forms/decidim/proposals/admin/proposal_form.rb b/decidim-proposals/app/forms/decidim/proposals/admin/proposal_form.rb
@@ -50,7 +50,7 @@ def category
         #
         # Returns a Decidim::Scope
         def scope
-          @scope ||= process_scope || organization_scopes.where(id: scope_id).first
+          @scope ||= organization_scopes.where(id: scope_id).first || process_scope
         end
       end
     end
diff --git a/decidim-proposals/app/forms/decidim/proposals/proposal_form.rb b/decidim-proposals/app/forms/decidim/proposals/proposal_form.rb
@@ -54,7 +54,7 @@ def category
       #
       # Returns a Decidim::Scope
       def scope
-        @scope ||= process_scope || organization_scopes.where(id: scope_id).first
+        @scope ||= organization_scopes.where(id: scope_id).first || process_scope
       end
 
       def has_address?
diff --git a/decidim-proposals/app/views/decidim/proposals/admin/proposals/_form.html.erb b/decidim-proposals/app/views/decidim/proposals/admin/proposals/_form.html.erb
@@ -26,7 +26,7 @@
 
     <% if current_participatory_process.has_subscopes? %>
       <div class="row column">
-        <%= form.collection_select :scope_id, subscopes_for(current_participatory_process), :id, :name %>
+        <%= form.scopes_select :scope_id, prompt: I18n.t("decidim.scopes.global"), remote_path: decidim.scopes_search_path(root: current_participatory_process.scope) %>
       </div>
     <% end %>
 
diff --git a/decidim-proposals/app/views/decidim/proposals/proposals/new.html.erb b/decidim-proposals/app/views/decidim/proposals/proposals/new.html.erb
@@ -40,7 +40,7 @@
 
           <% if current_participatory_process.has_subscopes? %>
             <div class="field">
-              <%= form.collection_select :scope_id, subscopes_for(current_participatory_process), :id, :name %>
+              <%= form.scopes_select :scope_id, prompt: I18n.t("decidim.scopes.global"), remote_path: decidim.scopes_search_path(root: current_participatory_process.scope) %>
             </div>
           <% end %>
 
diff --git a/decidim-proposals/spec/features/proposals_spec.rb b/decidim-proposals/spec/features/proposals_spec.rb
@@ -74,7 +74,7 @@
             fill_in :proposal_title, with: "Oriol for president"
             fill_in :proposal_body, with: "He will solve everything"
             select translated(category.name), from: :proposal_category_id
-            select translated(scope.name), from: :proposal_scope_id
+            select2 translated(scope.name), xpath: '//select[@id="proposal_scope_id"]/..', search: true
 
             find("*[type=submit]").click
           end
@@ -108,7 +108,7 @@
 
               fill_in :proposal_address, with: address
               select translated(category.name), from: :proposal_category_id
-              select translated(scope.name), from: :proposal_scope_id
+              select2 translated(scope.name), xpath: '//select[@id="proposal_scope_id"]/..', search: true
 
               find("*[type=submit]").click
             end
@@ -138,7 +138,7 @@
               fill_in :proposal_title, with: "Oriol for president"
               fill_in :proposal_body, with: "He will solve everything"
               select translated(category.name), from: :proposal_category_id
-              select translated(scope.name), from: :proposal_scope_id
+              select2 translated(scope.name), xpath: '//select[@id="proposal_scope_id"]/..', search: true
               select user_group.name, from: :proposal_user_group_id
 
               find("*[type=submit]").click
@@ -173,7 +173,7 @@
 
                 fill_in :proposal_address, with: address
                 select translated(category.name), from: :proposal_category_id
-                select translated(scope.name), from: :proposal_scope_id
+                select2 translated(scope.name), xpath: '//select[@id="proposal_scope_id"]/..', search: true
                 select user_group.name, from: :proposal_user_group_id
 
                 find("*[type=submit]").click
diff --git a/decidim-proposals/spec/shared/manage_proposals_examples.rb b/decidim-proposals/spec/shared/manage_proposals_examples.rb
@@ -60,7 +60,7 @@
               fill_in :proposal_title, with: "Make decidim great again"
               fill_in :proposal_body, with: "Decidim is great but it can be better"
               select translated(category.name), from: :proposal_category_id
-              select translated(scope.name), from: :proposal_scope_id
+              select2 translated(scope.name), xpath: '//select[@id="proposal_scope_id"]/..', search: true
 
               find("*[type=submit]").click
             end
diff --git a/decidim-results/app/forms/decidim/results/admin/result_form.rb b/decidim-results/app/forms/decidim/results/admin/result_form.rb
@@ -41,7 +41,7 @@ def process_scope
         end
 
         def scope
-          @scope ||= process_scope || organization_scopes.where(id: decidim_scope_id).first
+          @scope ||= organization_scopes.where(id: decidim_scope_id).first || process_scope
         end
 
         def category
diff --git a/decidim-results/app/views/decidim/results/admin/results/_form.html.erb b/decidim-results/app/views/decidim/results/admin/results/_form.html.erb
diff --git a/decidim-results/config/i18n-tasks.yml b/decidim-results/config/i18n-tasks.yml
diff --git a/decidim-results/spec/shared/manage_results_examples.rb b/decidim-results/spec/shared/manage_results_examples.rb

Original file line number	Diff line number	Diff line change
`@@ -82,7 +82,7 @@`
`82`	`82`	`)`
`83`	`83`	`fill_in :project_budget, with: 22_000_000`
`84`	`84`
`85`		`- select translated(scope.name), from: :project_decidim_scope_id`
	`85`	`+ select2 translated(scope.name), xpath: '//select[@id="project_decidim_scope_id"]/..', search: true`
`86`	`86`	`select translated(category.name), from: :project_decidim_category_id`
`87`	`87`
`88`	`88`	`find("*[type=submit]").click`