Skip to content

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also compare across forks.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also compare across forks.
...
  • 6 commits
  • 8 files changed
  • 0 commit comments
  • 1 contributor
Commits on Feb 07, 2013
@dregad dregad Fix #2971: add reminder events to the bug history 3d34fba
@dregad dregad Removed trailing ':' from reminder_sent_to language string
This is for consistency, as except for error messages punctuation is
generally added by the code.

Issue #2971
97eb148
@dregad dregad Improve handling of reminders' recipients list truncation
Replaced the previous method of truncating the list to a hardcoded
number of entries (50), to a more robust approach based on the size of
the underlying database field (250 chars, note_attr in bugnote table).

Added a message on the REMINDER bugnote, to inform user that the list of
recipients stored with the note, was actually truncated (Users should
refer to the issue's history to see who actually received the reminder).

This functionality relies on a hack, i.e. to indicate that the list was
truncated, bug_reminder.php is not storing the trailing delimiter in the
note_attr field, and this is picked up in bugnote_view_inc.php to
display the note to the user's attention.

Fixes #15470
4429c8f
@dregad dregad Optimize code to add reminder recipients to monitoring list 1913149
@dregad dregad Fix return value of email_bug_reminder()
The function now returns an array containing only the users to whom the
reminder e-mail was actually sent (i.e. excludes failures, blank e-mails,
etc).

In addition, the return value is now an array of user id's instead of
user names, which indeed makes more sense from an API perspective, as
mentioned by Julian Fitzell in 5ea3d0b,
since the function's only caller is bug_reminder.php which exclusively
relies on user id's.

Fixes #15472
389b2a6
@dregad dregad bug_reminder.php does not handle unsent reminders
Reminders are not logged in history (or the bugnote) if the e-mail was not
actually sent to the recipient.

The function now uses email_bug_reminder()'s return value instead of the
full user-provided list of recipients for inclusion in the reminder's
bugnote and to add history entries.

Fixes #15471
15db187
Showing with 70 additions and 32 deletions.
  1. +27 −13 bug_reminder.php
  2. +23 −6 bugnote_view_inc.php
  3. +1 −0 core/constant_inc.php
  4. +10 −10 core/email_api.php
  5. +4 −0 core/history_api.php
  6. +3 −1 lang/strings_english.txt
  7. +1 −1 print_all_bug_page_word.php
  8. +1 −1 print_bugnote_inc.php
View
40 bug_reminder.php
@@ -78,14 +78,18 @@
access_ensure_bug_level( config_get( 'bug_reminder_threshold' ), $f_bug_id );
-# Automically add recipients to monitor list if they are above the monitor
+# Automatically add recipients to monitor list if they are above the monitor
# threshold, option is enabled, and not reporter or handler.
-foreach ( $f_to as $t_recipient )
-{
- if ( ON == config_get( 'reminder_recipients_monitor_bug' ) &&
- access_has_bug_level( config_get( 'monitor_bug_threshold' ), $f_bug_id ) &&
- !bug_is_user_handler( $f_bug_id, $t_recipient ) &&
- !bug_is_user_reporter( $f_bug_id, $t_recipient ) ) {
+$t_reminder_recipients_monitor_bug = config_get( 'reminder_recipients_monitor_bug' );
+$t_monitor_bug_threshold = config_get( 'monitor_bug_threshold' );
+$t_handler = bug_get_field( $f_bug_id, 'handler_id' );
+$t_reporter = bug_get_field( $f_bug_id, 'reporter_id' );
+foreach ( $f_to as $t_recipient ) {
+ if ( ON == $t_reminder_recipients_monitor_bug
+ && access_has_bug_level( $t_monitor_bug_threshold, $f_bug_id )
+ && $t_recipient != $t_handler
+ && $t_recipient != $t_reporter
+ ) {
bug_monitor( $f_bug_id, $t_recipient );
}
}
@@ -94,17 +98,27 @@
# Add reminder as bugnote if store reminders option is ON.
if ( ON == config_get( 'store_reminders' ) ) {
- if ( count( $f_to ) > 50 ) { # too many recipients to log, truncate the list
- $t_to = array();
- for ( $i=0; $i<50; $i++ ) {
- $t_to[] = $f_to[$i];
+ # Build list of recipients, truncated to note_attr fields's length
+ $t_attr = '|';
+ $t_length = 0;
+ foreach( $result as $t_id ) {
+ $t_recipient = $t_id . '|';
+ $t_length += strlen( $t_recipient );
+ if( $t_length > 250 ) {
+ # Remove trailing delimiter to indicate truncation
+ $t_attr = rtrim( $t_attr, '|' );
+ break;
}
- $f_to = $t_to;
+ $t_attr .= $t_recipient;
}
- $t_attr = '|' . implode( '|', $f_to ) . '|';
bugnote_add( $f_bug_id, $f_body, 0, config_get( 'default_reminder_view_status' ) == VS_PRIVATE, REMINDER, $t_attr, NULL, FALSE );
}
+# Add history entries for all sent reminders
+foreach ( $result as $t_recipient ) {
+ history_log_event_special( $f_bug_id, BUG_REMINDER_SENT, $t_recipient );
+}
+
form_security_purge( 'bug_reminder' );
html_page_top( null, string_get_bug_view_url( $f_bug_id ) );
View
29 bugnote_view_inc.php
@@ -242,14 +242,31 @@
<?php
switch ( $t_bugnote->note_type ) {
case REMINDER:
- echo '<em>' . lang_get( 'reminder_sent_to' ) . ' ';
- $t_note_attr = utf8_substr( $t_bugnote->note_attr, 1, utf8_strlen( $t_bugnote->note_attr ) - 2 );
- $t_to = array();
- foreach ( explode( '|', $t_note_attr ) as $t_recipient ) {
- $t_to[] = prepare_user_name( $t_recipient );
+ echo '<em>';
+
+ # List of recipients; remove surrounding delimiters
+ $t_recipients = trim( $t_bugnote->note_attr, '|' );
+
+ if( empty( $t_recipients ) ) {
+ echo lang_get( 'reminder_sent_none' );
+ } else {
+ # If recipients list's last char is not a delimiter, it was truncated
+ $t_truncated = ( '|' != utf8_substr( $t_bugnote->note_attr, utf8_strlen( $t_bugnote->note_attr ) - 1 ) );
+
+ # Build recipients list for display
+ $t_to = array();
+ foreach ( explode( '|', $t_recipients ) as $t_recipient ) {
+ $t_to[] = prepare_user_name( $t_recipient );
+ }
+
+ echo lang_get( 'reminder_sent_to' ) . ': '
+ . implode( ', ', $t_to )
+ . ( $t_truncated ? ' (' . lang_get( 'reminder_list_truncated' ) . ')' : '' );
}
- echo implode( ', ', $t_to ) . '</em><br /><br />';
+
+ echo '</em><br /><br />';
break;
+
case TIME_TRACKING:
if ( access_has_bug_level( config_get( 'time_tracking_view_threshold' ), $f_bug_id ) ) {
echo '<div class="time-tracked">', lang_get( 'time_tracking_time_spent') . ' ' . $t_time_tracking_hhmm, '</div>';
View
1 core/constant_inc.php
@@ -189,6 +189,7 @@
define( 'TAG_RENAMED', 27 );
define( 'BUG_REVISION_DROPPED', 28 );
define( 'BUGNOTE_REVISION_DROPPED', 29 );
+define( 'BUG_REMINDER_SENT', 30 );
define( 'PLUGIN_HISTORY', 100 );
# bug revisions
View
20 core/email_api.php
@@ -812,7 +812,7 @@ function email_bug_deleted( $p_bug_id ) {
* @param string $p_subject
* @param string $p_message
* @param array $p_headers
- * @return int
+ * @return int e-mail queue id, or NULL if e-mail was not stored
*/
function email_store( $p_recipient, $p_subject, $p_message, $p_headers = null ) {
$t_recipient = trim( $p_recipient );
@@ -1134,14 +1134,12 @@ function email_append_domain( $p_email ) {
}
/**
- * Send a bug reminder to each of the given user, or to each user if the first parameter is an array
- * return an array of usernames to which the reminder was successfully sent
+ * Send a bug reminder to the given user(s), or to each user if the first parameter is an array
*
- * @todo I'm not sure this shouldn't return an array of user ids... more work for the caller but cleaner from an API point of view.
- * @param array $p_recipients
- * @param int $p_bug_id
- * @param string $p_message
- * @return null
+ * @param int|array $p_recipients user id or list of user ids array to send reminder to
+ * @param int $p_bug_id Issue for which the reminder is sent
+ * @param string $p_message Optional message to add to the e-mail
+ * @return array List of users ids to whom the reminder e-mail was actually sent
*/
function email_bug_reminder( $p_recipients, $p_bug_id, $p_message ) {
if( !is_array( $p_recipients ) ) {
@@ -1162,10 +1160,9 @@ function email_bug_reminder( $p_recipients, $p_bug_id, $p_message ) {
lang_push( user_pref_get_language( $t_recipient, $t_project_id ) );
$t_email = user_get_email( $t_recipient );
- $result[] = user_get_name( $t_recipient );
if( access_has_project_level( config_get( 'show_user_email_threshold' ), $t_project_id, $t_recipient ) ) {
- $t_sender_email = ' <' . current_user_get_field( 'email' ) . '>';
+ $t_sender_email = ' <' . user_get_email( $t_sender_id ) . '>';
} else {
$t_sender_email = '';
}
@@ -1174,6 +1171,9 @@ function email_bug_reminder( $p_recipients, $p_bug_id, $p_message ) {
if( ON == config_get( 'enable_email_notification' ) ) {
$t_id = email_store( $t_email, $t_subject, $t_contents );
+ if( $t_id !== null ) {
+ $result[] = $t_recipient;
+ }
log_event( LOG_EMAIL, "queued reminder email #$t_id for U$t_recipient" );
}
View
4 core/history_api.php
@@ -512,6 +512,10 @@ function history_localize_item( $p_field_name, $p_type, $p_old_value, $p_new_val
$p_old_value = user_get_name( $p_old_value );
$t_note = lang_get( 'bug_monitor' ) . ': ' . $p_old_value;
break;
+ case BUG_REMINDER_SENT:
+ $p_old_value = user_get_name( $p_old_value );
+ $t_note = lang_get( 'reminder_sent_to' ) . ': ' . $p_old_value;
+ break;
case BUG_UNMONITOR:
$p_old_value = user_get_name( $p_old_value );
$t_note = lang_get( 'bug_end_monitor' ) . ': ' . $p_old_value;
View
4 lang/strings_english.txt
@@ -470,7 +470,9 @@ If you requested this verification, visit the following URL to change your passw
'to' => 'To',
'sent_you_this_reminder_about' => 'sent you this reminder about',
'bug_reminder' => 'Send a reminder',
- 'reminder_sent_to' => 'Reminder sent to:',
+ 'reminder_sent_to' => 'Reminder sent to',
+ 'reminder_sent_none' => 'No reminders could be sent',
+ 'reminder_list_truncated' => 'recipients list truncated, refer to Issue History for full list',
'bug_send_button' => 'Send',
'reminder' => 'Reminder',
'reminder_explain' => 'This note will be sent to the recipients listed requesting feedback on this issue.',
View
2 print_all_bug_page_word.php
@@ -557,7 +557,7 @@
<?php
switch ( $t_bugnote->note_type ) {
case REMINDER:
- echo lang_get( 'reminder_sent_to' ) . ' ';
+ echo lang_get( 'reminder_sent_to' ) . ': ';
$t_note_attr = utf8_substr( $t_bugnote->note_attr, 1, utf8_strlen( $t_bugnote->note_attr ) - 2 );
$t_to = array();
foreach ( explode( '|', $t_note_attr ) as $t_recipient ) {
View
2 print_bugnote_inc.php
@@ -151,7 +151,7 @@
<?php
switch ( $v3_note_type ) {
case REMINDER:
- echo '<div class="italic">' . lang_get( 'reminder_sent_to' ) . ' ';
+ echo '<div class="italic">' . lang_get( 'reminder_sent_to' ) . ': ';
$v3_note_attr = utf8_substr( $v3_note_attr, 1, utf8_strlen( $v3_note_attr ) - 2 );
$t_to = array();
foreach ( explode( '|', $v3_note_attr ) as $t_recipient ) {

No commit comments for this range

Something went wrong with that request. Please try again.