Merge pull request #1087 from alphagov/472-changes-to-content-request…

…s-form-in-publisher

[DO NOT MERGE] Changes to content requests form in publisher

app/assets/stylesheets/application.scss

@@ -81,3 +81,7 @@ thead {
     background-color: $light-green;
   }
 }
+
+.hint-block--md-6 {
+  max-width: 555px;
+}

app/controllers/content_change_requests_controller.rb

@@ -16,11 +16,13 @@ def parse_request_from_params
   def content_change_request_params
     params.require(:support_requests_content_change_request).permit(
       :title,
+      :reason_for_change,
+      :subject_area,
       :details_of_change,
       :url,
       :related_urls,
       requester_attributes: %i[email name collaborator_emails],
-      time_constraint_attributes: %i[not_before_date needed_by_date time_constraint_reason],
+      time_constraint_attributes: %i[not_before_date needed_by_date time_constraint_reason needed_by_time not_before_time],
     ).to_h
   end
 end

app/models/support/requests/content_change_request.rb

@@ -5,7 +5,7 @@ module Requests
     class ContentChangeRequest < Request
       include WithTimeConstraint
 
-      attr_accessor :title, :details_of_change, :url, :related_urls
+      attr_accessor :title, :reason_for_change, :subject_area, :details_of_change, :url, :related_urls
 
       validates :details_of_change, presence: true
 
@@ -16,7 +16,7 @@ def initialize(attrs = {})
       end
 
       def self.label
-        "Content changes and new content requests"
+        "Request a content change or new content on GOV.UK"
       end
 
       def self.description

app/models/support/requests/time_constraint.rb

@@ -4,11 +4,28 @@ module Support
   module Requests
     class TimeConstraint
       include ActiveModel::Model
-      attr_accessor :not_before_date, :needed_by_date, :time_constraint_reason
+      attr_accessor :not_before_date, :not_before_time, :needed_by_date, :needed_by_time, :time_constraint_reason, :is_legal_deadline
 
-      validates_date :needed_by_date, allow_nil: true, allow_blank: true, on_or_after: :today
-      validates_date :not_before_date, allow_nil: true, allow_blank: true, on_or_after: :today
-      validates_date :not_before_date, on_or_before: :needed_by_date, unless: proc { |c| c.needed_by_date.blank? || c.not_before_date.blank? }
+      validates :is_legal_deadline, inclusion: %w[yes no], allow_blank: true
+
+      validates_date :needed_by_date, allow_blank: true, on_or_after: :today
+      validates_date :not_before_date, allow_blank: true, on_or_after: :today
+      validates_date :not_before_date, on_or_before: :needed_by_date, unless: proc { |c| c.needed_by_date.blank? || c.not_before_date.blank? },
+                                       message: "'Must not be published before' date cannot be after Deadline"
+
+      validates_time :not_before_time, allow_blank: true
+      validates_time :needed_by_time, allow_blank: true
+
+      validates_time :not_before_time, on_or_after: :now, unless: proc { |c|
+        c.not_before_time.blank? || c.not_before_date != Time.zone.today.strftime("%d-%m-%Y")
+      }
+      validates_time :needed_by_time, on_or_after: :now, unless: proc { |c|
+        c.needed_by_time.blank? || c.needed_by_date != Time.zone.today.strftime("%d-%m-%Y")
+      }
+
+      validates_time :not_before_time, on_or_before: :needed_by_time, unless: proc { |c|
+        [c.needed_by_date, c.needed_by_time, c.not_before_date, c.not_before_time].any?(&:blank?) || c.needed_by_date != c.not_before_date
+      }
     end
   end
 end

app/models/support/requests/with_time_constraint.rb

@@ -4,7 +4,9 @@ module WithTimeConstraint
       def self.included(base)
         base.validate do |request|
           if request.time_constraint && !request.time_constraint.valid?
-            errors.add :base, message: "Time constraint details are invalid."
+            request.time_constraint.errors.each do |error|
+              errors.add(error.attribute, error.message)
+            end
           end
         end
       end

app/models/zendesk/ticket/content_change_request_ticket.rb

@@ -19,13 +19,18 @@ def comment_snippets
         [
           Zendesk::LabelledSnippet.new(
             on: @request,
-            field: :url,
-            label: "URL of content to be changed",
+            field: :reason_for_change,
+            label: "Reason for change request",
           ),
           Zendesk::LabelledSnippet.new(
             on: @request,
-            field: :related_urls,
-            label: "Related URLs",
+            field: :subject_area,
+            label: "Subject area",
+          ),
+          Zendesk::LabelledSnippet.new(
+            on: @request,
+            field: :url,
+            label: "URLs to be changed",
           ),
           Zendesk::LabelledSnippet.new(
             on: @request,

app/models/zendesk/zendesk_ticket.rb

@@ -29,6 +29,18 @@ def needed_by_date
       end
     end
 
+    def not_before_time
+      if value?(:time_constraint) && value?(:not_before_time, @request.time_constraint)
+        @request.time_constraint.not_before_time
+      end
+    end
+
+    def needed_by_time
+      if value?(:time_constraint) && value?(:needed_by_time, @request.time_constraint)
+        @request.time_constraint.needed_by_time
+      end
+    end
+
     def tags
       %w[govt_form]
     end
@@ -49,7 +61,9 @@ def base_comment_snippets
       if value?(:time_constraint)
         [
           Zendesk::LabelledSnippet.new(on: self, field: :needed_by_date, label: "Needed by date"),
+          Zendesk::LabelledSnippet.new(on: self, field: :needed_by_time, label: "Needed by time"),
           Zendesk::LabelledSnippet.new(on: self, field: :not_before_date, label: "Not before date"),
+          Zendesk::LabelledSnippet.new(on: self, field: :not_before_time, label: "Not before time"),
           Zendesk::LabelledSnippet.new(on: @request.time_constraint, field: :time_constraint_reason, label: "Reason for time constraint"),
         ]
       else

app/views/content_change_requests/_request_details.html.erb

@@ -6,56 +6,150 @@
 
 <div class="form-group">
   <span class="form-label">
-    <%= f.label :title, "Title of request" %>
+    <%= f.label :title do %>
+      Title of request <abbr title="required">*</abbr>
+    <% end %>
   </span>
+  <p class="help-block hint-block--md-6" id="title-hint">
+    Clearly summarise what needs to change - for example, ‘Update phone numbers in Universal Credit guide’ or ‘Create a start page for clean air zones‘.
+  </p>
   <span class="form-wrapper">
-    <%= f.text_field :title, placeholder: "Content change request", class: "input-md-6 form-control" %>
+    <%= f.text_field :title, required: true, placeholder: "Content change request", class: "input-md-6 form-control",
+      aria: {
+        describedby: "title-hint"
+      }
+    %>
   </span>
 </div>
 
-<fieldset>
-  <legend>
-    <span>URL affected</span>
-  </legend>
+<div class="form-group">
+  <span class="form-label">
+    <%= f.label :reason_for_change do %>
+      What’s the reason for the request? <abbr title="required">*</abbr>
+    <% end %>
+  </span>
+  <p class="help-block hint-block--md-6" id="reason-hint">
+    If none of the options below are appropriate, please select other.
+  </p>
+  <span class="form-wrapper">
+    <%= f.select :reason_for_change,
+      options_for_select([
+        "Factual inaccuracy",
+        "Content is causing problems for users",
+        "New information (policy, campaign, service) - for example, service start page needed",
+        "Missing information",
+        "Update - policy or process change",
+        "Uprating - policy or money changes at the new tax year (6 April)",
+        "Service Review",
+        "Other"
+      ], f.object.reason_for_change),
+      { include_blank: true },
+      class: "input-md-6 form-control",
+      aria: {
+        describedby: "reason-hint"
+      },
+      required: true
+    %>
+  </span>
+</div>
 
-  <div class="form-group">
-    <span class="form-label">
-      <%= f.label :url, "URL" %>
-    </span>
-    <span class="form-wrapper">
-      <%= f.text_field :url, placeholder: "https://www.gov.uk/", class: "input-md-6 form-control" %>
-    </span>
-  </div>
+<div class="form-group">
+  <span class="form-label">
+    <%= f.label :subject_area do %>
+    What’s the subject area? <abbr title="required">*</abbr>
+    <% end%>
+  </span>
+  <p class="help-block hint-block--md-6" id="subject-area-hint">
+    If your request covers multiple subject areas, choose the one you think most relevant.
+  </p>
+  <span class="form-wrapper">
+    <%= f.select :subject_area,
+      options_for_select([
+        "Benefits",
+        "Births, death, marriages and care",
+        "Business and self-employed",
+        "Childcare and parenting",
+        "Citizenship and living in the UK",
+        "Crime, justice and the law",
+        "Disabled people",
+        "Driving and transport",
+        "Education and learning",
+        "Employing people",
+        "Environment and countryside",
+        "Health",
+        "Housing and local services",
+        "Money and tax",
+        "Passports, travel and living abroad",
+        "Visas and immigration",
+        "Working, jobs and pensions",
+        "Other"
+      ], f.object.subject_area),
+      { include_blank: true },
+      class: "input-md-6 form-control",
+      aria: {
+        describedby: "subject-area-hint"
+      },
+      required: true
+    %>
+  </span>
+</div>
+
+<div class="form-group">
+  <span class="form-label">
+    <%= f.label :url do %>
+      Which URLs are affected? <abbr title="required">*</abbr>
+    <% end %>
+  </span>
+  <p class="help-block hint-block--md-6" id="url-hint">
+    List URLs, one per line. Include any Welsh URLs where translations are needed.
+  </p>
+  <span class="form-wrapper">
+    <%= f.text_area :url, placeholder: "https://www.gov.uk/", required: true, class: "input-md-6 form-control", rows: 4, cols: 50, aria: {
+      describedby: "url-hint"
+    } %>
+  </span>
+</div>
 
   <div class="form-group">
     <span class="form-label">
       <%= f.label :details_of_change do %>
-        Details of the requested change<abbr title="required">*</abbr>
+        Tell us about the content that needs to be created, updated or is causing a problem for users? <abbr title="required">*</abbr>
       <% end %>
     </span>
-    <span class="form-wrapper">
-      <%= f.text_area :details_of_change, required: true, aria: { required: true }, class: "input-md-6 form-control", rows: 16, cols: 50 %>
-    </span>
+    <div class="help-block hint-block--md-6" id="details-of-change-hint">
+      <p>Make sure you:</p>
+      <ul>
+        <li>include evidence of why this is a problem, for example, user feedback or call centre data</li>
+        <li>give sufficient detail</li>
+        <li>use plain English</li>
+        <li>explain any acronyms, jargon or legal language</li>
+        <li>say which parts of the content are affected - for example, ‘first paragraph of overview’</li>
+        <li>separate different points - for example, by numbering them</li>
+      </ul>
+
+    <p>If you send us wording, we might not use it. If we do, we’ll need to put it in GOV.UK style.</p>
   </div>
+  <span class="form-wrapper">
+    <%= f.text_area :details_of_change, required: true, aria: { required: true }, class: "input-md-6 form-control", rows: 16, cols: 50, aria: {
+      describedby: "details-of-change-hint"
+    } %>
+  </span>
+</div>
 
-  <div class="form-group">
-    <span class="form-label">
-      <%= f.label :related_urls do %>
-        Does this affect any other URLs (including any existing Welsh translations that need to be updated)? Put each new URL on a new line.<abbr title="required">*</abbr>
-      <% end %>
-    </span>
-    <span class="form-wrapper">
-      <%= f.text_area :related_urls, required: true, aria: { required: true }, class: "input-md-6 form-control", rows: 4, cols: 50 %>
-    </span>
   </div>
-</fieldset>
 
 <%= render partial: "support/attachment_instructions" %>
 
 <%= render partial: "support/time_constraint", locals: { f: f } %>
 
 <div class="alert alert-info alert-block">
-  <p>If your request is urgent, please fill in this form and call <%= @primary_contact_details[:urgent_department_content_change][:phone] %> during office hours, or <%= @primary_contact_details[:urgent_mainstream_content_change][:phone] %> out of hours. (It's ONLY urgent when the public or the government is facing immediate and significant financial, legal or physical risk.)</p>
+  <p><strong>If your request is urgent</strong></p>
+  <p>Submit this form and call:</p>
+  <ul>
+    <li><%= @primary_contact_details[:urgent_department_content_change][:phone] %> between 9am and 6pm</li>
+    <li><%= @primary_contact_details[:urgent_mainstream_content_change][:phone] %> at other times</li>
+  </ul>
+  <p>It's urgent only if the public or the government faces immediate and significant financial, legal or physical risk.</p>
 </div>
 
 <%= render partial: "support/collaborators", locals: { f: f } %>

app/views/layouts/application.html.erb

@@ -37,7 +37,11 @@
       data-module="auto-track-event"
       data-track-action="alert-<%= alert_class %>"
       data-track-label="<%= strip_tags(flash[k]) %>">
-        <%= flash[k] %>
+      <ul>
+        <% flash[k].split("\\n").each do |message| %>
+          <li><%= message %> </li>
+        <% end %>
+      </ul>
     </div>
   <% end %>
 

app/views/support/_collaborators.html.erb

@@ -1,10 +1,15 @@
 <%= f.fields_for :requester do |r| %>
   <div class="form-group">
     <span class="form-label">
-      <%= r.label :collaborator_emails, "Should anybody else be copied in on this request? (comma-separated list of emails)" %>
+      <%= r.label :collaborator_emails, "Who needs a copy of this request?" %>
     </span>
+    <p class="help-block hint-block--md-6" id="collaborators-hint">
+      Separate email addresses with commas.
+    </p>
     <span class="form-wrapper">
-      <%= r.text_field :collaborator_emails, required: false, class: "input-md-6 form-control", value: r.object.collaborator_emails.join(", ") %>
+      <%= r.text_field :collaborator_emails, required: false, class: "input-md-6 form-control", value: r.object.collaborator_emails.join(", "), aria: {
+        describedby: "collaborators-hint"
+      } %>
     </span>
   </div>
 <% end %>