Merge pull request #6296 from opf/fix/27604/columns-output

Add columns to query_params_representer

[ci skip]

app/helpers/types_helper.rb

@@ -109,7 +109,10 @@ module ::TypesHelper
     # Remove the templated filter since we can't yet handle it in the frontend
     query.filters.delete_if(&:templated?)
 
-    ::API::V3::Queries::QueryParamsRepresenter.new(query).to_h
+    # Modify the hash to match Rails array based +to_query+ transforms:
+    # e.g., { columns: [1,2] }.to_query == "columns[]=1&columns[]=2" (unescaped)
+    # The frontend will do that IFF the hash key is an array
+    ::API::V3::Queries::QueryParamsRepresenter.new(query).to_json
   end
 
   def attr_form_map(key, represented)

app/views/types/form/_form_configuration.html.erb

@@ -147,7 +147,7 @@ See docs/COPYRIGHT.rdoc for more details.
         <div id="draggable-groups" dragula='"groups"'>
           <% form_attributes[:actives].each do |group, attributes| %>
             <% if group.is_a? ::Type::QueryGroup %>
-              <%= render partial: 'types/form/query_group', locals: { group: group, query: attributes } %>
+              <%= render partial: 'types/form/query_group', locals: { group: group, query_json: attributes } %>
             <% else %>
               <%= render partial: 'types/form/attribute_group', locals: { group: group, attributes: attributes } %>
             <% end %>

app/views/types/form/_query_group.html.erb

@@ -16,7 +16,7 @@
   </div>
   <%= content_tag :div,
                   class: 'type-form-query',
-                  data: { query: JSON.dump(query) } do %>
+                  data: { query: query_json } do %>
     <span class="type-form-query-group--edit-button" ng-click="editQuery($event)">
       <%= op_icon('button--icon icon-edit') %>
       <%= t('types.edit.edit_query') %>

frontend/app/components/modals/export-modal/wp-table-export.modal.ts

@@ -67,7 +67,12 @@ export class WpTableExportModal extends OpModalComponent implements OnInit {
       return column.id;
     });
 
-    return href + "&" + this.UrlParamsHelper.buildQueryString({ 'columns[]': columnIds });
+    let url = URI(href);
+    // Remove current columns
+    url.removeSearch('columns[]');
+    url.addSearch('columns[]', columnIds);
+
+    return url.toString();
   }
 
   protected get afterFocusOn():JQuery {

lib/api/v3/queries/query_params_representer.rb

@@ -37,16 +37,35 @@ module API
   module V3
     module Queries
       class QueryParamsRepresenter
+
         def initialize(query)
           self.query = query
         end
 
-        def to_h
+        ##
+        # To json hash outputs the hash to be parsed to the frontend http
+        # which contains a reference to the columns array as columns[].
+        # This will match the Rails +to_query+ output
+        def to_json
+          to_h(column_key: 'columns[]'.to_sym).to_json
+        end
+
+        ##
+        # Output as query params used for directly using in URL queries.
+        # Outputs columns[]=A,columns[]=B due to Rails query output.
+        def to_url_query(merge_params: {})
+          to_h
+            .merge(merge_params)
+            .to_query
+        end
+
+        def to_h(column_key: :columns)
           p = default_hash
 
           p[:showSums] = 'true' if query.display_sums?
           p[:groupBy] = query.group_by if query.group_by?
           p[:sortBy] = sort_criteria_to_v3 if query.sorted?
+          p[column_key] = columns_to_v3 unless query.has_default_columns?
 
           # an empty filter param is also relevant as this would mean to not apply
           # the default filter (status - open)
@@ -65,6 +84,10 @@ module API
 
         private
 
+        def columns_to_v3
+          query.column_names.map { |name| convert_to_v3(name) }
+        end
+
         def sort_criteria_to_v3
           converted = query.sort_criteria.map { |first, last| [convert_to_v3(first), last] }
 

lib/api/v3/queries/query_representer.rb

@@ -67,10 +67,9 @@ module API
 
           url_query = ::API::V3::Queries::QueryParamsRepresenter
                       .new(represented)
-                      .to_h
-                      .merge(params.slice(:offset, :pageSize))
+                      .to_url_query(merge_params: params.slice(:offset, :pageSize))
           {
-            href: [path, url_query.to_query].join('?')
+            href: [path, url_query].join('?')
           }
         end
 

spec/features/types/form_configuration_query_spec.rb

@@ -87,8 +87,47 @@ describe 'form subelements configuration', type: :feature, js: true do
       query = type_bug.attribute_groups.detect { |x| x.key == 'Columns Test' }
       expect(query).to be_present
 
-      columns = query.attributes.columns.map(&:name).sort
-      expect(columns).to eq(%i[id subject])
+      column_names = query.attributes.columns.map(&:name).sort
+      expect(column_names).to eq %i[id subject]
+
+      form.add_query_group('Second query')
+      form.edit_query_group('Second query')
+
+      # Restrict filters to type_task
+      modal.switch_to 'Columns'
+
+      columns.assume_opened
+      columns.uncheck_all save_changes: false
+      columns.add 'ID', save_changes: false
+      columns.apply
+
+      # Save changed query
+      form.save_changes
+      expect(page).to have_selector('.flash.notice', text: 'Successful update.', wait: 10)
+
+      type_bug.reload
+      query = type_bug.attribute_groups.detect { |x| x.key == 'Columns Test' }
+      expect(query).to be_present
+
+      column_names = query.attributes.columns.map(&:name).sort
+      expect(column_names).to eq %i[id subject]
+
+      query = type_bug.attribute_groups.detect { |x| x.key == 'Second query' }
+      expect(query).to be_present
+
+      column_names = query.attributes.columns.map(&:name).sort
+      expect(column_names).to eq %i[id]
+
+      form.edit_query_group('Second query')
+      modal.switch_to 'Columns'
+      columns.expect_checked 'ID'
+      columns.apply
+
+      form.edit_query_group('Columns Test')
+      modal.switch_to 'Columns'
+      columns.expect_checked 'ID'
+      columns.expect_checked 'Subject'
+      columns.apply
     end
 
     it 'can create and save embedded subelements' do

spec/support/components/work_packages/columns.rb

@@ -76,6 +76,12 @@ module Components
         apply if save_changes
       end
 
+      def expect_checked(name)
+        within_modal do
+          expect(page).to have_selector("input[type=checkbox][title='#{name}']")
+        end
+      end
+
       def uncheck_all(save_changes: true)
         modal_open? or open_modal