Updated UX based on review feedback

- added linked checkbox to make initial_message_len change more explicit - added basic styling to visually group related form elements in submission prefs form - moved SI guideline for initial message lengths to underneath textarea - styled SI message length guideline similarly to SI file size guideline - updated codename error flash message to display on 2 lines
freedomofpress · Mar 8, 2022 · e5a565c · e5a565c
1 parent 90d3541
commit e5a565c
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 35 deletions.
diff --git a/securedrop/journalist_app/admin.py b/securedrop/journalist_app/admin.py
@@ -15,7 +15,7 @@
 from db import db
 from html import escape
 from models import (InstanceConfig, Journalist, InvalidUsernameException,
-                    FirstOrLastNameError, PasswordError)
+                    FirstOrLastNameError, PasswordError, Submission)
 from journalist_app.decorators import admin_required
 from journalist_app.utils import (commit_account_changes, set_diceware_password,
                                   validate_hotp_secret, revoke_token)
@@ -36,9 +36,15 @@ def index() -> str:
     @view.route('/config', methods=('GET', 'POST'))
     @admin_required
     def manage_config() -> Union[str, werkzeug.Response]:
-        # The UI prompt ("prevent") is the opposite of the setting ("allow"):
+        # The UI document upload prompt ("prevent") is the opposite of the setting ("allow")
+        if InstanceConfig.get_default().initial_message_min_len > 0:
+            prevent_short_messages = True
+        else:
+            prevent_short_messages = False
+
         submission_preferences_form = SubmissionPreferencesForm(
             prevent_document_uploads=not InstanceConfig.get_default().allow_document_uploads,
+            prevent_short_messages=prevent_short_messages,
             min_message_length=InstanceConfig.get_default().initial_message_min_len,
             reject_codename_messages=InstanceConfig.get_default().reject_message_with_codename
             )
@@ -70,6 +76,7 @@ def manage_config() -> Union[str, werkzeug.Response]:
             return render_template("config.html",
                                    submission_preferences_form=submission_preferences_form,
                                    organization_name_form=organization_name_form,
+                                   max_len=Submission.MAX_MESSAGE_LEN,
                                    logo_form=logo_form)
 
     @view.route('/update-submission-preferences', methods=['POST'])
@@ -80,16 +87,19 @@ def update_submission_preferences() -> Optional[werkzeug.Response]:
             # The UI prompt ("prevent") is the opposite of the setting ("allow"):
             allow_uploads = not form.prevent_document_uploads.data
 
-            try:
-                msg_length = form.min_message_length.data
-
-                if isinstance(msg_length, int):
-                    int_length = msg_length
-                elif isinstance(msg_length, str):
-                    int_length = int(msg_length)
-                else:
+            if form.prevent_short_messages.data:
+                try:
+                    msg_length = form.min_message_length.data
+
+                    if isinstance(msg_length, int):
+                        int_length = msg_length
+                    elif isinstance(msg_length, str):
+                        int_length = int(msg_length)
+                    else:
+                        int_length = 0
+                except ValueError:
                     int_length = 0
-            except ValueError:
+            else:
                 int_length = 0
 
             reject_codenames = form.reject_codename_messages.data

diff --git a/securedrop/journalist_app/forms.py b/securedrop/journalist_app/forms.py
@@ -131,10 +131,16 @@ class SubmissionPreferencesForm(FlaskForm):
                                             'prevent_document_uploads',
                                             false_values=('false', 'False', '')
                                            )
-    min_message_length = IntegerField('min_message_length', validators=[check_message_length],
+    prevent_short_messages = BooleanField(
+                                            'prevent_short_messages',
+                                            false_values=('false', 'False', '')
+                                           )
+    min_message_length = IntegerField('min_message_length',
+                                      validators=[
+                                          RequiredIf('prevent_short_messages'),
+                                          check_message_length],
                                       render_kw={
-                                          'aria-describedby': 'message-length-notes',
-                                          'class': 'short_int_field'})
+                                          'aria-describedby': 'message-length-notes'})
     reject_codename_messages = BooleanField(
                                             'reject_codename_messages',
                                             false_values=('false', 'False', '')

diff --git a/securedrop/journalist_templates/config.html b/securedrop/journalist_templates/config.html
@@ -55,22 +55,27 @@ <h2 id="config-preventuploads">{{ gettext('Submission Preferences') }}</h2>
 
   <form action="{{ url_for('admin.update_submission_preferences') }}" method="post">
     <input name="csrf_token" type="hidden" value="{{ csrf_token() }}">
+
       <div class="config_form_element">
         {{ submission_preferences_form.prevent_document_uploads() }}
         <label
       for="prevent_document_uploads">{{ gettext('Prevent sources from uploading documents. Sources will still be able to send messages.') }}</label>
       </div>
 
       <div class="config_form_element">
-        {{ submission_preferences_form.min_message_length() }}
+        {{ submission_preferences_form.prevent_short_messages() }}
         <label
-      for="min_message_length">{{ gettext('Minimum message length in chars for sources\' first submission if no document included. Set to 0 to disable.') }}</label>
+      for="prevent_short_messages">{{ gettext('Prevent sources from sending initial messages shorter than the minimum required length:') }}</label>
+          <div class="config_form_subelement">
+            {{ submission_preferences_form.min_message_length(min=0, max=max_len) }}
+            <label class="form-field-hint"
+          for="min_message_length">{{ gettext('Minimum number of characters.') }}</label>
+          </div>
       </div>
-
       <div class="config_form_element">
         {{ submission_preferences_form.reject_codename_messages() }}
         <label
-      for="reject_codename_messages">{{ gettext('Prevent sources from submitting their codename as a message') }}</label>
+      for="reject_codename_messages">{{ gettext('Prevent sources from submitting their codename as an initial message.') }}</label>
       </div>
     <div class="section-spacing">
       <button type="submit" id="submit-submission-preferences" class="icon icon-edit"

diff --git a/securedrop/sass/journalist.sass b/securedrop/sass/journalist.sass
@@ -16,6 +16,21 @@
 .content div
   align-content: center
 
+.config_form_subelement
+  margin-left: 1.5em
+  margin-bottom: 0
+
+.config_form_element
+  padding: 3px
+  margin-bottom: 1.0em
+
+#min_message_length
+  width: 6em
+
+.form-field-hint
+  font-size: 0.9em
+  font-style: italic
+
 #replies
   .reply
     border: 1px solid $color_grey_medium
@@ -203,7 +218,7 @@ button.button-star.un-starred
     background-image: url(/static/icons/unstarred.png)
     width: 16px
     height: 15px
-  
+
   &:active, &:hover
     span:before
       background-image: url(/static/icons/starred.png)
@@ -232,8 +247,8 @@ button.icon-reset:not([data-tooltip]), button.icon-reset[data-tooltip] > span.la
     width: 15px
     height: 15px
 
-.icon-bell 
+.icon-bell
   &:before
     background-image: url(/static/icons/bell.png)
     width: 13px
-    height: 15px
+    height: 15px
diff --git a/securedrop/sass/source.sass b/securedrop/sass/source.sass
@@ -255,6 +255,11 @@ p#max-file-size
   font-style: italic
   margin-bottom: 0
 
+p#min-msg-length
+  font-size: 10px
+  font-style: italic
+  margin-bottom: 0
+
 .bubble
   width: 415px
   position: absolute

diff --git a/securedrop/source_app/main.py b/securedrop/source_app/main.py
@@ -7,7 +7,7 @@
 from typing import Union
 
 import werkzeug
-from flask import (Blueprint, render_template, flash, redirect, url_for,
+from flask import (Blueprint, render_template, flash, redirect, url_for, escape,
                    session, current_app, request, Markup, abort, g, make_response)
 from flask_babel import gettext
 
@@ -204,16 +204,17 @@ def submit(logged_in_source: SourceUser) -> werkzeug.Response:
             min_len = InstanceConfig.get_default().initial_message_min_len
             if (min_len > 0) and (msg and not fh) and (len(msg) < min_len):
                 flash(gettext(
-                    "Your initial message must be at least {} characters long.".format(min_len)),
+                    "Your first message must be at least {} characters long.".format(min_len)),
                     "error")
                 return redirect(f"{url_for('main.lookup')}#flashed")
 
             codenames_rejected = InstanceConfig.get_default().reject_message_with_codename
             if codenames_rejected and codename_detected(msg, session['new_user_codename']):
-                flash(gettext(
-                    "Please do not submit your codename! Keep it secret, and"
-                    " use it to log in later to check for replies."),
-                     "error")
+                flash(Markup('{}<br>{}'.format(
+                    escape(gettext("Please do not submit your codename!")),
+                    escape(gettext("Keep your codename secret, and use it to log in later"
+                                   " to check for replies."))
+                    )), "error")
                 return redirect(f"{url_for('main.lookup')}#flashed")
 
         if not os.path.exists(Storage.get_default().path(logged_in_source.filesystem_id)):

diff --git a/securedrop/source_templates/lookup.html b/securedrop/source_templates/lookup.html
@@ -18,14 +18,8 @@
   {% if allow_document_uploads %}
   <h2 id="submit-heading" class="headline">{{ gettext('Submit Files or Messages') }}</h2>
   <p class="explanation">{{ gettext('You can submit any kind of file, a message, or both.') }}</p>
-      {% if min_len > 0 %}
-      <p class="explanation">{{ gettext('If your initial submission is message-only, it must be at least {} characters long.'.format(min_len)) }}</p>
-      {% endif %}
   {% else %}
   <h2 id="submit-heading" class="headline">{{ gettext('Submit Messages') }}</h2>
-      {% if min_len > 0 %}
-      <p class="explanation">{{ gettext('Your initial message must be at least {} characters long.'.format(min_len)) }}</p>
-      {% endif %}
   {% endif %}
 
   <p class="explanation extended-explanation">
@@ -49,6 +43,15 @@ <h2 id="submit-heading" class="headline">{{ gettext('Submit Messages') }}</h2>
       {% endif %}
       <div class="message grid-item{% if not allow_document_uploads %} wide{% endif %}">
         {{ form.msg(class="fill-parent") }}
+  {% if allow_document_uploads %}
+      {% if min_len > 0 %}
+         <p class="center" id="min-msg-length">{{ gettext('If you are only sending a message, it must be at least {} characters long.'.format(min_len)) }}</p>
+      {% endif %}
+  {% else %}
+      {% if min_len > 0 %}
+        <p class="center" id="min-msg-length">{{ gettext('Your first message must be at least {} characters long.'.format(min_len)) }}</p>
+      {% endif %}
+  {% endif %}
       </div>
     </div>
 
@@ -57,8 +60,6 @@ <h2 id="submit-heading" class="headline">{{ gettext('Submit Messages') }}</h2>
         {{ gettext('SUBMIT') }}
       </button>
 
-
-
       <a href="{{ url_for('main.lookup') }}" class="btn secondary" id="cancel" role="button"
         aria-label="{{ gettext('Cancel') }}">
         {{ gettext('CANCEL') }}