From 5025f89d61398ca91d25d28d273a90c6d1da99f2 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Fri, 27 Sep 2024 10:14:48 +0100
Subject: [PATCH 1/3] Update unstyled button override

Fix hover background removal.
---
 app/assets/stylesheets/responsive/_global_style.scss | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/assets/stylesheets/responsive/_global_style.scss b/app/assets/stylesheets/responsive/_global_style.scss
index 119065eead..304a078b78 100644
--- a/app/assets/stylesheets/responsive/_global_style.scss
+++ b/app/assets/stylesheets/responsive/_global_style.scss
@@ -191,7 +191,7 @@ form button {
   padding: 2px 6px 3px;
   transition: none;
   &:hover, &:focus {
-    background: none;
+    background-color: inherit;
     color: inherit;
   }
 }

From 898e19bd9cc53a5f2f4791d680408bbaed8b25fa Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Thu, 26 Sep 2024 17:36:39 +0100
Subject: [PATCH 2/3] Update project edit question step

1. Improve templates and CSS.
2. Fix prevention autocompletion of form changes when reloading page.
3. Switch to buttons instead of anchors with no href.
---
 .../alaveteli_pro/_projects_layout.scss       | 46 +++++++++++++------
 .../projects/key_set_controller.js            |  4 ++
 .../alaveteli_pro/projects/_key_set.html.erb  | 41 +++++++++--------
 .../projects/edit_key_set.html.erb            |  6 +--
 .../projects/update_key_set.turbo_stream.erb  |  2 +-
 5 files changed, 61 insertions(+), 38 deletions(-)

diff --git a/app/assets/stylesheets/responsive/alaveteli_pro/_projects_layout.scss b/app/assets/stylesheets/responsive/alaveteli_pro/_projects_layout.scss
index 3bd4c4cb65..72946d8ce1 100644
--- a/app/assets/stylesheets/responsive/alaveteli_pro/_projects_layout.scss
+++ b/app/assets/stylesheets/responsive/alaveteli_pro/_projects_layout.scss
@@ -294,41 +294,57 @@
 }
 
 #project-key-set {
-  .project-key-set__key {
-    &.project-key-set__key--removed {
-      display: none;
-    }
+  padding-bottom: 1em;
 
-    width: 75%;
+  .project-key-set__key {
+    padding: 1em;
+    margin-bottom: 1em;
 
     display: flex;
     flex-flow: row nowrap;
     justify-content: space-between;
 
+    border-radius: 5px;
+    box-shadow: rgba(0, 0, 0, 0.05) 0px 6px 24px 0px, rgba(0, 0, 0, 0.08) 0px 0px 0px 1px;
+
+    &.project-key-set__key--removed {
+      display: none;
+    }
+
     .project-key-set__key__drag-handle {
       text-indent: -9999px;
       background-image: image-url("drag.svg");
       background-repeat: no-repeat;
-      background-position: center center;
+      background-position: center 2em;
       cursor: grab;
       width: 24px;
     }
 
-    .project-key-set__key__title,
-    .project-key-set__key__format {
-      width: 35%;
-      input,
-      select {
-        width: 100%;
-      }
+    .project-key-set__key__attributes {
+      width: 100%;
+      margin: 0 1em;
+
+      display: flex;
+      flex-flow: row wrap;
+      justify-content: space-between;
+
+      .project-key-set__key__value {
+        width: 50%;
+        padding-right: 1em;
+
+        input[type='text'], select {
+          width: 100%;
+        }
 
-      .fieldWithErrors {
-        padding: 0;
+        .fieldWithErrors {
+          padding: 0;
+        }
       }
     }
 
     .project-key-set__key__remove-question {
       padding-top: 1.2em;
+      button { width: max-content; }
     }
   }
 }
diff --git a/app/javascript/controllers/projects/key_set_controller.js b/app/javascript/controllers/projects/key_set_controller.js
index 483161623e..cd63b811de 100644
--- a/app/javascript/controllers/projects/key_set_controller.js
+++ b/app/javascript/controllers/projects/key_set_controller.js
@@ -26,6 +26,10 @@ export default class extends Controller {
     streamUpdate(this.element);
   }
 
+  updateKey(event) {
+    streamUpdate(this.element);
+  }
+
   updateOrder() {
     this.element
       .querySelectorAll('[name*="[order]"]')
diff --git a/app/views/alaveteli_pro/projects/_key_set.html.erb b/app/views/alaveteli_pro/projects/_key_set.html.erb
index 71a242ca1d..632a1433e8 100644
--- a/app/views/alaveteli_pro/projects/_key_set.html.erb
+++ b/app/views/alaveteli_pro/projects/_key_set.html.erb
@@ -1,29 +1,34 @@
-<%= f.fields_for :key_set, @key_set do |key_set_fields| %>
-  <%= key_set_fields.fields_for :keys, @keys do |key_fields| %>
-    <div class="row project-key-set__key<% if key_fields.object.marked_for_destruction? %> project-key-set__key--removed<% end %>">
+<%= form.fields_for :key_set, @key_set do |key_set_fields| %>
+  <%= key_set_fields.fields_for :keys, @keys do |f| %>
+    <% key = f.object %>
+    <div class="row project-key-set__key<% if key.marked_for_destruction? %> project-key-set__key--removed<% end %>">
       <div class="project-key-set__key__drag-handle">
-        <%= key_fields.hidden_field :order %>
+        <%= f.hidden_field :order %>
         Drag to reorder
       </div>
 
-      <div class="project-key-set__key__title">
-        <%= key_fields.label :title %>
-        <%= key_fields.text_field :title, autocomplete: false %>
-      </div>
+      <div class="project-key-set__key__attributes">
+        <div class="project-key-set__key__value">
+          <%= f.label :title %>
+          <%= f.text_field :title, autocomplete: 'off' %>
+        </div>
 
-      <div class="project-key-set__key__format">
-        <%= key_fields.label :format %>
-        <%= key_fields.select :format,
-          options_for_select(
-            Dataset::Key.format_options,
-            key_fields.object.format
-          ), {}, autocomplete: false %>
+        <div class="project-key-set__key__value">
+          <%= f.label :format %>
+          <%= f.select :format,
+            options_for_select(
+              Dataset::Key.format_options,
+              key.format
+            ),
+            {},
+            data: { action: 'change->projects--key-set#updateKey' },
+            autocomplete: 'off' %>
+        </div>
       </div>
 
       <div class="project-key-set__key__remove-question">
-        <%= key_fields.hidden_field :_destroy %>
-        <%= link_to _('Remove question'), nil, class: 'button-tertiary',
-          data: { action: 'click->projects--key-set#removeKey' } %>
+        <%= f.hidden_field :_destroy %>
+        <%= button_tag _('Remove question'), class: 'button-tertiary', data: { action: 'click->projects--key-set#removeKey' } %>
       </div>
     </div>
   <% end %>
diff --git a/app/views/alaveteli_pro/projects/edit_key_set.html.erb b/app/views/alaveteli_pro/projects/edit_key_set.html.erb
index 2c92547c7f..8b9d8da037 100644
--- a/app/views/alaveteli_pro/projects/edit_key_set.html.erb
+++ b/app/views/alaveteli_pro/projects/edit_key_set.html.erb
@@ -20,13 +20,11 @@
         <%= hidden_field_tag :step, current_step %>
 
         <div id="project-key-set" data-projects--key-set-target="keySet">
-          <%= render partial: 'key_set', locals: { f: f } %>
+          <%= render partial: 'key_set', locals: { form: f } %>
         </div>
 
         <div>
-          <%= link_to _('Add question'), nil, class: 'button-tertiary', data: {
-            action: 'click->projects--key-set#addKey'
-          } %>
+          <%= button_tag _('Add question'), class: 'button-tertiary', data: { action: 'click->projects--key-set#addKey' } %>
         </div>
 
         <div class="form_button">
diff --git a/app/views/alaveteli_pro/projects/update_key_set.turbo_stream.erb b/app/views/alaveteli_pro/projects/update_key_set.turbo_stream.erb
index b915ba316c..ecbffe8880 100644
--- a/app/views/alaveteli_pro/projects/update_key_set.turbo_stream.erb
+++ b/app/views/alaveteli_pro/projects/update_key_set.turbo_stream.erb
@@ -1,6 +1,6 @@
 <%= form_for @project, html: { id: 'temp' } do |f| %>
   <div id="project-key-set">
-    <%= turbo_stream.update 'project-key-set', partial: 'key_set', locals: { f: f } %>
+    <%= turbo_stream.update 'project-key-set', partial: 'key_set', locals: { form: f } %>
   </div>
 <% end %>
 

From e6fccc28a7a51666e41dd91ac03171542eddf9e5 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Fri, 27 Sep 2024 10:16:59 +0100
Subject: [PATCH 3/3] Add project dataset select format

Allow project owners to add questions with a pre set list of options
which can be selected when extracting information from requests.

There are also options to allow blank value or multiple values, these
effect how the form select input is rendered in the extracting sidebar.

Fixes #8117
---
 .../alaveteli_pro/projects_controller.rb      |  6 ++-
 .../projects/extracts_controller.rb           |  2 +-
 .../projects/key_set/select_controller.js     | 24 +++++++++++
 app/models/dataset/key.rb                     | 16 +++++++-
 app/models/dataset/value.rb                   |  9 ++++
 .../alaveteli_pro/projects/_key_set.html.erb  |  5 +++
 .../projects/_select_key.html.erb             | 41 +++++++++++++++++++
 .../dataset/keys/_select_key.html.erb         |  4 ++
 ...240926164308_add_options_to_dataset_key.rb |  5 +++
 doc/CHANGES.md                                |  1 +
 spec/factories/dataset_keys.rb                |  7 +++-
 spec/models/dataset/key_spec.rb               | 13 +++++-
 spec/models/dataset/value_spec.rb             | 41 +++++++++++++++++++
 13 files changed, 168 insertions(+), 6 deletions(-)
 create mode 100644 app/javascript/controllers/projects/key_set/select_controller.js
 create mode 100644 app/views/alaveteli_pro/projects/_select_key.html.erb
 create mode 100644 app/views/projects/dataset/keys/_select_key.html.erb
 create mode 100644 db/migrate/20240926164308_add_options_to_dataset_key.rb

diff --git a/app/controllers/alaveteli_pro/projects_controller.rb b/app/controllers/alaveteli_pro/projects_controller.rb
index d4c589aede..93cdbbfbe7 100644
--- a/app/controllers/alaveteli_pro/projects_controller.rb
+++ b/app/controllers/alaveteli_pro/projects_controller.rb
@@ -141,7 +141,11 @@ def project_params
     when 'edit_key_set', 'update_key_set'
       params.fetch(:project, {}).permit(
         key_set_attributes: [
-          :id, keys_attributes: %i[id title format order _destroy]
+          :id, keys_attributes: [
+            :id, :title, :format, :order, :_destroy, options: [
+              :select_allow_blank, :select_allow_muliple, { select_options: [] }
+            ]
+          ]
         ]
       )
     when 'edit_contributors', 'update_contributors'
diff --git a/app/controllers/projects/extracts_controller.rb b/app/controllers/projects/extracts_controller.rb
index 9cc041ab24..1014ebb90e 100644
--- a/app/controllers/projects/extracts_controller.rb
+++ b/app/controllers/projects/extracts_controller.rb
@@ -71,7 +71,7 @@ def find_info_request
 
   def extract_params
     params.require(:extract).permit(
-      :dataset_key_set_id, values_attributes: [:dataset_key_id, :value]
+      :dataset_key_set_id, values_attributes: [:dataset_key_id, :value, value: []]
     )
   end
 
diff --git a/app/javascript/controllers/projects/key_set/select_controller.js b/app/javascript/controllers/projects/key_set/select_controller.js
new file mode 100644
index 0000000000..11b02b356a
--- /dev/null
+++ b/app/javascript/controllers/projects/key_set/select_controller.js
@@ -0,0 +1,24 @@
+import { Controller } from "@hotwired/stimulus";
+import { streamUpdate } from "helpers/stream_update";
+
+export default class extends Controller {
+  static targets = ["newInput"];
+  static values = { name: String };
+
+  addOption(event) {
+    event.preventDefault();
+
+    const newValue = this.newInputTarget.value;
+    if (!newValue) return;
+
+    const form = this.element.closest("form");
+    streamUpdate(form, { [this.nameValue]: newValue });
+  }
+
+  removeOption(event) {
+    event.preventDefault();
+
+    const li = event.target.closest("li");
+    if (li) li.remove();
+  }
+}
diff --git a/app/models/dataset/key.rb b/app/models/dataset/key.rb
index f48db23ce0..2d4aef091f 100644
--- a/app/models/dataset/key.rb
+++ b/app/models/dataset/key.rb
@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 20210114161442
+# Schema version: 20240926164308
 #
 # Table name: dataset_keys
 #
@@ -10,6 +10,7 @@
 #  order              :integer
 #  created_at         :datetime         not null
 #  updated_at         :datetime         not null
+#  options            :jsonb
 #
 
 ##
@@ -24,6 +25,7 @@ class Dataset::Key < ApplicationRecord
 
   FORMATS = {
     text: { title: _('Text'), regexp: /\A.*\z/m },
+    select: { title: _('Select'), regexp: /\A.*\z/m },
     numeric: { title: _('Numeric'), regexp: /\A[0-9,%\+\-\s]*\z/ },
     boolean: { title: _('Yes/No'), regexp: /\A(0|1)\z/ }
   }.freeze
@@ -40,4 +42,16 @@ def self.format_options
   def format_regexp
     FORMATS[format.to_sym][:regexp]
   end
+
+  def select_options
+    options.fetch('select_options', [])
+  end
+
+  def select_allow_blank?
+    options.fetch('select_allow_blank', '0').to_i == 1
+  end
+
+  def select_allow_muliple?
+    options.fetch('select_allow_muliple', '0').to_i == 1
+  end
 end
diff --git a/app/models/dataset/value.rb b/app/models/dataset/value.rb
index 735b99a3b0..c1488009cb 100644
--- a/app/models/dataset/value.rb
+++ b/app/models/dataset/value.rb
@@ -29,6 +29,15 @@ def title
     key.title
   end
 
+  def value=(new_value)
+    case key.format
+    when 'select'
+      super(Array(new_value).reject(&:blank?).join(', '))
+    else
+      super
+    end
+  end
+
   def mapped_value
     case key.format
     when 'boolean'
diff --git a/app/views/alaveteli_pro/projects/_key_set.html.erb b/app/views/alaveteli_pro/projects/_key_set.html.erb
index 632a1433e8..aa8184285f 100644
--- a/app/views/alaveteli_pro/projects/_key_set.html.erb
+++ b/app/views/alaveteli_pro/projects/_key_set.html.erb
@@ -24,6 +24,11 @@
             data: { action: 'change->projects--key-set#updateKey' },
             autocomplete: 'off' %>
         </div>
+
+        <% if lookup_context.exists?("alaveteli_pro/projects/_#{key.format}_key") %>
+          <%= render partial: "alaveteli_pro/projects/#{key.format}_key",
+                    locals: { form: f, key: key } %>
+        <% end %>
       </div>
 
       <div class="project-key-set__key__remove-question">
diff --git a/app/views/alaveteli_pro/projects/_select_key.html.erb b/app/views/alaveteli_pro/projects/_select_key.html.erb
new file mode 100644
index 0000000000..a60b9947fe
--- /dev/null
+++ b/app/views/alaveteli_pro/projects/_select_key.html.erb
@@ -0,0 +1,41 @@
+<%= form.fields_for :options, key.options do |f| %>
+  <div class="project-key-set__key__value">
+    <%= f.label :select_allow_blank do %>
+      <%= f.check_box :select_allow_blank, checked: key.select_allow_blank?, autocomplete: 'off' %>
+      <%= _('Allow blank') %>
+    <% end %>
+  </div>
+
+  <div class="project-key-set__key__value">
+    <%= f.label :select_allow_muliple do %>
+      <%= f.check_box :select_allow_muliple, checked: key.select_allow_muliple?, autocomplete: 'off' %>
+      <%= _('Allow muliple') %>
+    <% end %>
+  </div>
+
+  <%= content_tag(:div, class: "project-key-set__key__value", data: {
+    controller: "projects--key-set--select",
+    projects__key_set__select_name_value: "#{f.object_name}[select_options][]"
+  }) do %>
+    <%= f.label :select_options, _('Options') %>
+    <ul>
+      <% key.select_options.each_with_index do |option, index| %>
+        <li>
+          <%= option %>
+          <%= hidden_field_tag "#{f.object_name}[select_options][]", option %>
+          <%= button_tag class: 'button-unstyled', data: { action: 'click->projects--key-set--select#removeOption' } do %>
+            <%= _('Remove') %>
+          <% end %>
+        </li>
+      <% end %>
+    </ul>
+    <div class="row collapse postfix-radius">
+      <div class="small-10 columns">
+        <%= text_field_tag "select_option", nil, data: { projects__key_set__select_target: 'newInput' } %>
+      </div>
+      <div class="small-2 columns">
+        <%= button_tag _('Add'), class: 'button-tertiary postfix', data: { action: 'click->projects--key-set--select#addOption' } %>
+      </div>
+    </div>
+  <% end %>
+<% end %>
diff --git a/app/views/projects/dataset/keys/_select_key.html.erb b/app/views/projects/dataset/keys/_select_key.html.erb
new file mode 100644
index 0000000000..cfebb4a910
--- /dev/null
+++ b/app/views/projects/dataset/keys/_select_key.html.erb
@@ -0,0 +1,4 @@
+<div class="dataset-key">
+  <%= f.label :value, key.title %>
+  <%= f.select :value, key.select_options, { include_blank: key.select_allow_blank? }, multiple: key.select_allow_muliple? %>
+</div>
diff --git a/db/migrate/20240926164308_add_options_to_dataset_key.rb b/db/migrate/20240926164308_add_options_to_dataset_key.rb
new file mode 100644
index 0000000000..2ef98b5b0d
--- /dev/null
+++ b/db/migrate/20240926164308_add_options_to_dataset_key.rb
@@ -0,0 +1,5 @@
+class AddOptionsToDatasetKey < ActiveRecord::Migration[7.0]
+  def change
+    add_column :dataset_keys, :options, :jsonb, default: {}
+  end
+end
diff --git a/doc/CHANGES.md b/doc/CHANGES.md
index 116e1ab02d..d23e713bb7 100644
--- a/doc/CHANGES.md
+++ b/doc/CHANGES.md
@@ -2,6 +2,7 @@
 
 ## Highlighted Features
 
+* Add project dataset question "select" format (Graeme Porteous)
 * Fix script/mailin when multiple EXCEPTION_NOTIFICATIONS_TO addresses are
   specified (Graeme Porteous)
 * Add example logrotate configuration (Graeme Porteous)
diff --git a/spec/factories/dataset_keys.rb b/spec/factories/dataset_keys.rb
index 8af52b242f..3a39856d12 100644
--- a/spec/factories/dataset_keys.rb
+++ b/spec/factories/dataset_keys.rb
@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 20210114161442
+# Schema version: 20240926164308
 #
 # Table name: dataset_keys
 #
@@ -10,6 +10,7 @@
 #  order              :integer
 #  created_at         :datetime         not null
 #  updated_at         :datetime         not null
+#  options            :jsonb
 #
 
 FactoryBot.define do
@@ -24,6 +25,10 @@
       format { 'text' }
     end
 
+    trait :select do
+      format { 'select' }
+    end
+
     trait :numeric do
       format { 'numeric' }
     end
diff --git a/spec/models/dataset/key_spec.rb b/spec/models/dataset/key_spec.rb
index e266abadb4..8352caf8b4 100644
--- a/spec/models/dataset/key_spec.rb
+++ b/spec/models/dataset/key_spec.rb
@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 20210114161442
+# Schema version: 20240926164308
 #
 # Table name: dataset_keys
 #
@@ -10,6 +10,7 @@
 #  order              :integer
 #  created_at         :datetime         not null
 #  updated_at         :datetime         not null
+#  options            :jsonb
 #
 
 require 'spec_helper'
@@ -63,6 +64,8 @@
       is_expected.not_to be_valid
       key.format = 'text'
       is_expected.to be_valid
+      key.format = 'select'
+      is_expected.to be_valid
       key.format = 'numeric'
       is_expected.to be_valid
       key.format = 'boolean'
@@ -75,7 +78,8 @@
 
     it 'returns title/format key hash' do
       is_expected.to eq(
-        { 'Text' => :text, 'Numeric' => :numeric, 'Yes/No' => :boolean }
+        { 'Text' => :text, 'Select' => :select, 'Numeric' => :numeric,
+          'Yes/No' => :boolean }
       )
     end
   end
@@ -88,6 +92,11 @@
       it { is_expected.to eq described_class::FORMATS[:text][:regexp] }
     end
 
+    context 'select format' do
+      let(:key) { FactoryBot.build(:dataset_key, :select) }
+      it { is_expected.to eq described_class::FORMATS[:select][:regexp] }
+    end
+
     context 'numeric format' do
       let(:key) { FactoryBot.build(:dataset_key, :numeric) }
       it { is_expected.to eq described_class::FORMATS[:numeric][:regexp] }
diff --git a/spec/models/dataset/value_spec.rb b/spec/models/dataset/value_spec.rb
index 938a056309..a33c42758f 100644
--- a/spec/models/dataset/value_spec.rb
+++ b/spec/models/dataset/value_spec.rb
@@ -81,4 +81,45 @@ def invalid(value_to_test)
       end
     end
   end
+
+  describe 'value=' do
+    context 'when format is a select' do
+      before do
+        value.key = FactoryBot.build(:dataset_key, :select)
+      end
+
+      it 'accept an array' do
+        value.value = %w[a b]
+        expect(value.value).to eq('a, b')
+      end
+
+      it 'ignores empty array values' do
+        value.value = %w[a] + [nil] + %w[c]
+        expect(value.value).to eq('a, c')
+      end
+
+      it 'accepts as string' do
+        value.value = 'a'
+        expect(value.value).to eq('a')
+      end
+    end
+  end
+
+  describe 'mapped_value' do
+    context 'when format is a boolean' do
+      before do
+        value.key = FactoryBot.build(:dataset_key, :boolean)
+      end
+
+      it 'returns No for value 0' do
+        value.value = 0
+        expect(value.mapped_value).to eq('No')
+      end
+
+      it 'returns Yes for value 1' do
+        value.value = 1
+        expect(value.mapped_value).to eq('Yes')
+      end
+    end
+  end
 end