[Rt-commit] rt branch, 4.4/missing-attachment-warning, created. rt-4.2.11-5-g1e0ccce
Shawn Moore
shawn at bestpractical.com
Mon May 11 17:23:54 EDT 2015
The branch, 4.4/missing-attachment-warning has been created
at 1e0ccce67abc5c66e8076ea4bfc89426fc087b5d (commit)
- Log -----------------------------------------------------------------
commit 073661afaed8cef1cc43e60476a7f493a254a061
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Mon May 11 21:13:46 2015 +0000
Add .attachment to each tr of the attachments list
This way we can inspect them with jQuery
diff --git a/share/html/Ticket/Elements/AddAttachments b/share/html/Ticket/Elements/AddAttachments
index 7c1f0b8..769b691 100644
--- a/share/html/Ticket/Elements/AddAttachments
+++ b/share/html/Ticket/Elements/AddAttachments
@@ -46,7 +46,7 @@
%#
%# END BPS TAGGED BLOCK }}}
% if ( $attachments ) {
-<tr><td class="label"><&|/l&>Attached file</&>:</td>
+<tr class="attachment"><td class="label"><&|/l&>Attached file</&>:</td>
<td>
<&|/l&>Check box to delete</&><br />
% foreach my $attach_name ( sort keys %$attachments ) {
commit 1e0ccce67abc5c66e8076ea4bfc89426fc087b5d
Author: Shawn M Moore <shawn at bestpractical.com>
Date: Mon May 11 21:15:40 2015 +0000
Flag mentions of "attachment" when there are none
This is an imperfect (but hopefully good-enough) solution to the common
problem of forgetting to add attachments to tickets. There are lots of
corner cases handled (like warning when you add an attachment but later
check its ticky box to delete it) but there are a few that are less
tractable (like when you quote a transaction that mentioned the word
"attachment"—which disables this feature—and then forget to add your
own attachment in reply)
This feature does not forbid you from creating tickets or anything like
that. It merely provides a hint message in the UI.
Fixes: I#30951
diff --git a/share/html/Elements/JavascriptConfig b/share/html/Elements/AttachmentWarning
similarity index 66%
copy from share/html/Elements/JavascriptConfig
copy to share/html/Elements/AttachmentWarning
index 9437567..cea5612 100644
--- a/share/html/Elements/JavascriptConfig
+++ b/share/html/Elements/AttachmentWarning
@@ -45,40 +45,15 @@
%# those contributions and any derivatives thereof.
%#
%# END BPS TAGGED BLOCK }}}
-<%init>
-my $Config = {};
-$Config->{$_} = RT->Config->Get( $_, $session{CurrentUser} )
- for qw(rtname WebPath MessageBoxRichTextHeight);
+<div class="messagebox-attachment-warning">
+ <div class="ignore">
+ <% $QuotedMessage %>
+ <% $Signature %>
+ </div>
-my $CurrentUser = {};
-if ($session{CurrentUser} and $session{CurrentUser}->id) {
- $CurrentUser->{$_} = $session{CurrentUser}->$_
- for qw(id Name EmailAddress RealName);
-
- $CurrentUser->{Privileged} = $session{CurrentUser}->Privileged
- ? JSON::true : JSON::false;
-
- $Config->{WebHomePath} = RT->Config->Get("WebPath")
- . (!$session{CurrentUser}->Privileged ? "/SelfService" : "");
-}
-
-my $Catalog = {
- quote_in_filename => "Filenames with double quotes can not be uploaded.", #loc
-};
-$_ = loc($_) for values %$Catalog;
-
-$m->callback(
- CallbackName => "Data",
- CurrentUser => $CurrentUser,
- Config => $Config,
- Catalog => $Catalog,
-);
-</%init>
-<script>
-window.RT = {};
-RT.CurrentUser = <% JSON( $CurrentUser ) |n%>;
-RT.Config = <% JSON( $Config ) |n%>;
-
-RT.I18N = {};
-RT.I18N.Catalog = <% JSON( $Catalog ) |n %>;
-</script>
+ <p><&|/l&>It looks like you may have forgotten to add an attachment.</&></p>
+</div>
+<%ARGS>
+$QuotedMessage => ''
+$Signature => ''
+</%ARGS>
diff --git a/share/html/Elements/JavascriptConfig b/share/html/Elements/JavascriptConfig
index 9437567..7385dd3 100644
--- a/share/html/Elements/JavascriptConfig
+++ b/share/html/Elements/JavascriptConfig
@@ -64,6 +64,7 @@ if ($session{CurrentUser} and $session{CurrentUser}->id) {
my $Catalog = {
quote_in_filename => "Filenames with double quotes can not be uploaded.", #loc
+ attachment_warning_regex => "\\battach", #loc
};
$_ = loc($_) for values %$Catalog;
diff --git a/share/html/Elements/MessageBox b/share/html/Elements/MessageBox
index bcc64d4..da2e3ea 100644
--- a/share/html/Elements/MessageBox
+++ b/share/html/Elements/MessageBox
@@ -50,6 +50,11 @@
% $m->callback( %ARGS, SignatureRef => \$signature );
<% $Default || '' %><% $message %><% $signature %></textarea>
% $m->callback( %ARGS, CallbackName => 'AfterTextArea' );
+
+% if (!$SuppressAttachmentWarning) {
+% $m->comp('/Elements/AttachmentWarning', QuotedMessage => $message, Signature => $signature, %ARGS);
+% }
+
% if ($Type eq 'text/html') {
<input type="text" style="display:none" name="<% $Name %>Type" id="<% $Name %>Type" value="<% $m->request_args->{$Name."Type"}||$Type %>" />
% }
@@ -91,12 +96,13 @@ if ($Width) {
</%INIT>
<%ARGS>
-$QuoteTransaction => undef
-$Name => 'Content'
-$Default => ''
-$Width => RT->Config->Get('MessageBoxWidth', $session{'CurrentUser'} )
-$Height => RT->Config->Get('MessageBoxHeight', $session{'CurrentUser'} ) || 15
-$IncludeSignature => RT->Config->Get('MessageBoxIncludeSignature');
-$IncludeArticle => 1;
-$Type => RT->Config->Get('MessageBoxRichText', $session{'CurrentUser'}) ? 'text/html' : 'text/plain';
+$QuoteTransaction => undef
+$Name => 'Content'
+$Default => ''
+$Width => RT->Config->Get('MessageBoxWidth', $session{'CurrentUser'} )
+$Height => RT->Config->Get('MessageBoxHeight', $session{'CurrentUser'} ) || 15
+$IncludeSignature => RT->Config->Get('MessageBoxIncludeSignature');
+$IncludeArticle => 1;
+$Type => RT->Config->Get('MessageBoxRichText', $session{'CurrentUser'}) ? 'text/html' : 'text/plain';
+$SuppressAttachmentWarning => 0
</%ARGS>
diff --git a/share/html/Ticket/Forward.html b/share/html/Ticket/Forward.html
index 2970298..ee2c295 100644
--- a/share/html/Ticket/Forward.html
+++ b/share/html/Ticket/Forward.html
@@ -76,9 +76,9 @@
<td><&|/l&>Content</&>:</td>
<td>
% if (exists $ARGS{Content}) {
-<& /Elements/MessageBox, Default => $ARGS{Content}, IncludeSignature => 0 &>
+<& /Elements/MessageBox, Default => $ARGS{Content}, IncludeSignature => 0, SuppressAttachmentWarning => 1 &>
% } else {
-<& /Elements/MessageBox &>
+<& /Elements/MessageBox, SuppressAttachmentWarning => 1 &>
%}
</td>
</tr>
diff --git a/share/html/m/ticket/create b/share/html/m/ticket/create
index 957e829..59e8daf 100644
--- a/share/html/m/ticket/create
+++ b/share/html/m/ticket/create
@@ -229,7 +229,7 @@ $showrows->(
loc("Subject") => '<input type="text" name="Subject" size="30" maxsize="200" value="'.$escape->($ARGS{Subject} || '').'" />');
</%perl>
<span class="content-label label"><%loc("Describe the issue below")%></span>
- <& /Elements/MessageBox, exists $ARGS{Content} ? (Default => $ARGS{Content}, IncludeSignature => 0 ) : ( QuoteTransaction => $QuoteTransaction ), Height => 5 &>
+ <& /Elements/MessageBox, exists $ARGS{Content} ? (Default => $ARGS{Content}, IncludeSignature => 0 ) : ( QuoteTransaction => $QuoteTransaction ), Height => 5, SuppressAttachmentWarning => 1 &>
<&/Elements/Submit, Label => loc("Create") &>
diff --git a/share/html/m/ticket/reply b/share/html/m/ticket/reply
index 9a92150..e9f5b9c 100644
--- a/share/html/m/ticket/reply
+++ b/share/html/m/ticket/reply
@@ -109,12 +109,12 @@
% # preserve QuoteTransaction so we can use it to set up sane references/in/reply to
% my $temp = $ARGS{'QuoteTransaction'};
% delete $ARGS{'QuoteTransaction'};
-<& /Elements/MessageBox, Name=>"UpdateContent", Default=>$ARGS{UpdateContent}, IncludeSignature => 0, %ARGS&>
+<& /Elements/MessageBox, Name=>"UpdateContent", Default=>$ARGS{UpdateContent}, IncludeSignature => 0, SuppressAttachmentWarning => 1, %ARGS&>
% $ARGS{'QuoteTransaction'} = $temp;
% } else {
% my $IncludeSignature = 1;
% $IncludeSignature = 0 if $Action ne 'Respond' && !RT->Config->Get('MessageBoxIncludeSignatureOnComment');
-<& /Elements/MessageBox, Name=>"UpdateContent", IncludeSignature => $IncludeSignature, %ARGS &>
+<& /Elements/MessageBox, Name=>"UpdateContent", IncludeSignature => $IncludeSignature, SuppressAttachmentWarning => 1, %ARGS &>
% }
</div></div>
diff --git a/share/static/css/base/forms.css b/share/static/css/base/forms.css
index 8c6bb29..5e2c565 100644
--- a/share/static/css/base/forms.css
+++ b/share/static/css/base/forms.css
@@ -238,6 +238,14 @@ form div.submit div.buttons div.next {
color: red;
}
+.messagebox-attachment-warning {
+ display: none;
+}
+
+.messagebox-attachment-warning .ignore {
+ display: none;
+}
+
/* query builder */
#formatbuttons {
diff --git a/share/static/css/rudder/forms.css b/share/static/css/rudder/forms.css
index a10da2b..22d17cc 100644
--- a/share/static/css/rudder/forms.css
+++ b/share/static/css/rudder/forms.css
@@ -102,3 +102,14 @@ button {
.value {
font-size: 1em
}
+
+.messagebox-attachment-warning {
+ background-color: #fcc;
+ font-weight: bold;
+ padding: .5em 2em;
+ margin: .5em 0;
+
+ -moz-border-radius: 3px;
+ -webkit-border-radius: 3px;
+ border-radius: 3px;
+}
diff --git a/share/static/js/util.js b/share/static/js/util.js
index b665c0e..38f309a 100644
--- a/share/static/js/util.js
+++ b/share/static/js/util.js
@@ -260,6 +260,120 @@ function ReplaceAllTextareas() {
}
};
+function AddAttachmentWarning() {
+ var plainMessageBox = jQuery('.messagebox');
+ var addFileField = jQuery('input[name=Attach]');
+ var existingFileList = jQuery('tr.attachment input[type=checkbox]');
+ var warningMessage = jQuery('.messagebox-attachment-warning');
+ var ignoreMessage = warningMessage.find('.ignore');
+
+ // there won't be a ckeditor when using the plain <textarea>
+ var richTextEditor;
+ var messageBoxId = plainMessageBox.attr('id');
+ if (CKEDITOR.instances && CKEDITOR.instances[messageBoxId]) {
+ richTextEditor = CKEDITOR.instances[messageBoxId];
+ }
+
+ var regex = new RegExp(loc_key("attachment_warning_regex"), "i");
+
+ // if the quoted text or signature contains the magic word
+ // then we can't do much here, because the user can make any text
+ // changes they want and there's no real way to track the provenance of
+ // the word "attachment"
+ var ignoreMessageText = ignoreMessage.text();
+ if (ignoreMessageText && ignoreMessageText.match(regex)) {
+ return;
+ }
+
+ // a true value for instant means no CSS animation, for displaying the
+ // warning at page load time
+ var toggleAttachmentWarning = function (instant) {
+ var text;
+ if (richTextEditor) {
+ text = richTextEditor.getData();
+ }
+ else {
+ text = plainMessageBox.val();
+ }
+
+ // if the word "attach" appears and there are no attachments in flight
+ var needsWarning = text &&
+ text.match(regex) &&
+ !addFileField.val() &&
+ existingFileList.filter(":not(:checked)").length == 0;
+
+ if (needsWarning) {
+ warningMessage.show(instant ? 1 : 'fast');
+ }
+ else {
+ warningMessage.hide(instant ? 1 : 'fast');
+ }
+ };
+
+ // don't run all the machinery (including regex matching a potentially very
+ // long message) several times per keystroke
+ var timer;
+ var delayedAttachmentWarning = function () {
+ if (timer) {
+ return;
+ }
+
+ timer = setTimeout(function () {
+ timer = 0;
+ toggleAttachmentWarning();
+ }, 200);
+ };
+
+ if (richTextEditor) {
+ richTextEditor.on('instanceReady', function () {
+ // this set of events is imperfect. what I really want is:
+ // this.on('change', ...)
+ // but ckeditor doesn't seem to provide that out of the box
+
+ this.on('blur', function () {
+ toggleAttachmentWarning();
+ });
+
+ // we want to capture ~every keystroke type event; we only do the
+ // full checking periodically to avoid overloading the browser
+ this.document.on("keyup", function () {
+ delayedAttachmentWarning();
+ });
+ this.document.on("keydown", function () {
+ delayedAttachmentWarning();
+ });
+ this.document.on("keypress", function () {
+ delayedAttachmentWarning();
+ });
+
+ // hook into the undo/redo buttons in the ckeditor UI
+ this.getCommand('undo').on('afterUndo', function () {
+ toggleAttachmentWarning();
+ });
+ this.getCommand('redo').on('afterRedo', function () {
+ toggleAttachmentWarning();
+ });
+ });
+ }
+ else {
+ // the propertychange event is for IE
+ plainMessageBox.bind('input propertychange', function () {
+ delayedAttachmentWarning();
+ });
+ }
+
+ addFileField.bind('change', function () {
+ toggleAttachmentWarning();
+ });
+
+ existingFileList.bind('change', function () {
+ toggleAttachmentWarning();
+ });
+
+ // also need to display the warning on initial page load
+ toggleAttachmentWarning(1);
+}
+
function toggle_addprincipal_validity(input, good, title) {
if (good) {
jQuery(input).nextAll(".warning").hide();
@@ -342,4 +456,5 @@ jQuery(function() {
});
});
ReplaceAllTextareas();
+ AddAttachmentWarning();
});
-----------------------------------------------------------------------
More information about the rt-commit
mailing list