[feature/attachment-management-no-reassignment] Further feature adjustments.

-add database update entry;
- add files statistics checks;
- improve files deletion procedure;
- improve reversed order for pagination;
- adjust template file;
- add missing comma for orphan key in info module;
- change module mode name;
- fix module explanation text;
- add files comments output;
- change attachments per page amount from posts to topics per page value.

PHPBB3-9721

phpBB/adm/style/acp_attachments.html

@@ -371,7 +371,7 @@
 	</fieldset>
 	</form>
 
-<!-- ELSEIF S_ATTACHMENTS -->
+<!-- ELSEIF S_MANAGE -->
 
 	<form id="attachments" method="post" action="{U_ACTION}">
 
@@ -388,41 +388,34 @@
 	<thead>
 	<tr>
 		<th>{L_FILENAME}</th>
-		<th>{L_FILEDATE}</th>
+		<th>{L_POSTED}</th>
 		<th>{L_FILESIZE}</th>
-		<th>{L_ATTACH_POST_ID}</th>
 		<th>{L_DELETE}</th>
 	</tr>
 	</thead>
 	<tbody>
 	<!-- BEGIN attachments -->
 		<!-- IF attachments.S_ROW_COUNT is even --><tr class="row1"><!-- ELSE --><tr class="row2"><!-- ENDIF -->
-			<td><a href="{attachments.U_FILE}" style="font-weight: bold;">{attachments.REAL_FILENAME}</a><br />{attachments.L_DOWNLOAD_COUNT}<br /><!-- IF attachments.S_IN_MESSAGE -->{L_IN} {L_PRIVATE_MESSAGE}<!-- ELSE -->{L_TOPIC}: <!-- ENDIF --><a href="{attachments.U_VIEW_TOPIC}">{attachments.TOPIC_TITLE}</a></td>
+			<td><a href="{attachments.U_FILE}" style="font-weight: bold;">{attachments.REAL_FILENAME}</a><br /><!-- IF attachments.COMMENT -->{attachments.COMMENT}<br /><!-- ENDIF -->{attachments.L_DOWNLOAD_COUNT}<br /><!-- IF attachments.S_IN_MESSAGE -->{L_IN} {L_PRIVATE_MESSAGE}<!-- ELSE -->{L_TOPIC}: <a href="{attachments.U_VIEW_TOPIC}">{attachments.TOPIC_TITLE}</a><!-- ENDIF --></td>
 			<td>{attachments.FILETIME}<br />{L_POST_BY_AUTHOR} {attachments.ATTACHMENT_POSTER}</td>
 			<td>{attachments.FILESIZE}</td>
-			<td>
-				<input type="text" name="post_id[{attachments.ATTACH_ID}]" size="7" maxlength="10" value="{attachments.POST_ID}" {attachments.DISABLED} />
-				<input type="hidden" name="current_post_id[{attachments.ATTACH_ID}]" value="{attachments.POST_ID}" />
-				<input type="hidden" name="current_topic_id[{attachments.ATTACH_ID}]" value="{attachments.TOPIC_ID}" />
-			</td>
 			<td><input type="checkbox" class="radio" name="delete[{attachments.ATTACH_ID}]" /></td>
 		</tr>
 	<!-- END attachments -->
 	<tr class="row4">
-		<td colspan="4">&nbsp;</td>
+		<td colspan="3">&nbsp;</td>
 		<td class="small"><a href="#" onclick="marklist('attachments', 'delete', true); return false;">{L_MARK_ALL}</a> :: <a href="#" onclick="marklist('attachments', 'delete', false); return false;">{L_UNMARK_ALL}</a></td>
 	</tr>
 	</tbody>
 	</table>
 
+	<!-- IF TOTAL_FILES -->
 	<fieldset class="display-options">
 		{L_DISPLAY_LOG}: &nbsp;{S_LIMIT_DAYS}&nbsp;{L_SORT_BY}: {S_SORT_KEY} {S_SORT_DIR}
 		<input class="button2" type="submit" value="{L_GO}" name="sort" />
-		{S_FORM_TOKEN}
 	</fieldset>
 	<hr />
 
-	<!-- IF PAGINATION  or TOTAL_FILES -->
 	<div class="pagination">
 		{L_NUMBER_FILES}: {TOTAL_FILES} &bull; {L_TOTAL_SIZE}: {TOTAL_SIZE}<!-- IF S_ON_PAGE --><!-- IF PAGINATION --> &bull; <a href="#" onclick="jumpto(); return false;" title="{L_JUMP_TO_PAGE}">{S_ON_PAGE}</a> &bull; <span>{PAGINATION}</span><!-- ELSE --> &bull; {S_ON_PAGE}<!-- ENDIF --><!-- ENDIF -->
 	</div>
@@ -434,7 +427,7 @@
 		<input class="button1" type="submit" id="submit" name="submit" value="{L_SUBMIT}" />&nbsp;
 		<input class="button2" type="reset" id="reset" name="reset" value="{L_RESET}" />
 	</p>
-
+	{S_FORM_TOKEN}
 	</fieldset>
 
 	</form>

phpBB/includes/acp/acp_attachments.php

@@ -61,7 +61,7 @@ class acp_attachments
 				$l_title = 'ACP_ORPHAN_ATTACHMENTS';
 			break;
 
-			case 'attachments':
+			case 'manage':
 				$l_title = 'ACP_MANAGE_ATTACHMENTS';
 			break;
 
@@ -1048,7 +1048,7 @@ class acp_attachments
 
 			break;
 
-			case 'attachments':
+			case 'manage':
 
 				if ($submit)
 				{
@@ -1067,14 +1067,25 @@ class acp_attachments
 							$deleted_filenames[] = $row['real_filename'];
 						}
 						$db->sql_freeresult($result);
-						delete_attachments('attach', $delete_files);
+
+						if ($num_deleted = delete_attachments('attach', $delete_files))
+						{
+							if (sizeof($delete_files) != $num_deleted)
+							{
+								$error[] = $user->lang['FILES_GONE'];
+							}
 							add_log('admin', 'LOG_ATTACHMENTS_DELETED', implode(', ', $deleted_filenames));
 							$notify[] = sprintf($user->lang['LOG_ATTACHMENTS_DELETED'], implode(', ', $deleted_filenames));
 						}
+						else
+						{
+							$error[] = $user->lang['NO_FILES_TO_DELETE'];
+						}
+					}
 				}
 
 				$template->assign_vars(array(
-					'S_ATTACHMENTS'		=> true)
+					'S_MANAGE'		=> true)
 				);
 
 				$start		= request_var('start', 0);
@@ -1096,26 +1107,48 @@ class acp_attachments
 				$limit_filetime = ($min_filetime) ? " AND a.filetime >= $min_filetime " : '';
 				$start = ($sort_days && isset($_POST['sort'])) ? 0 : $start;
 
+				$attachments_per_page = (int) $config['topics_per_page'];
+
 				$num_files = (int) $config['num_files'];
-				$total_size = get_formatted_filesize((int) $config['upload_dir_size']);
+				$total_size = (int) $config['upload_dir_size'];
+
+				// Check if files statistics is accurate
+				$sql = 'SELECT COUNT(attach_id) as num_files
+					FROM ' . ATTACHMENTS_TABLE . '
+					WHERE is_orphan = 0';
+				$result = $db->sql_query($sql, 600);
+				$num_files_real = (int) $db->sql_fetchfield('num_files');
+				$db->sql_freeresult($result);
+
+				$sql = 'SELECT SUM(filesize) as upload_dir_size
+					FROM ' . ATTACHMENTS_TABLE . '
+					WHERE is_orphan = 0';
+				$result = $db->sql_query($sql, 600);
+				$total_size_real = (int) $db->sql_fetchfield('upload_dir_size');
+				$db->sql_freeresult($result);
+
+				if (($num_files != $num_files_real) || ($total_size != $total_size_real))
+				{
+					$error[] = sprintf($user->lang['FILES_STAT_WRONG'], $num_files_real, get_formatted_filesize($total_size_real));
+				}
 
 				// Make sure $start is set to the last page if it exceeds the amount
 				if ($start < 0 || $start > $num_files)
 				{
-					$start = ($start < 0) ? 0 : floor(($num_files - 1) / $config['posts_per_page']) * $config['posts_per_page'];
+					$start = ($start < 0) ? 0 : floor(($num_files - 1) / $attachments_per_page) * $attachments_per_page;
 				}
 
 				// If the user is trying to reach the second half of the attachments list, fetch it starting from the end
 				$store_reverse = false;
-				$sql_limit = $config['posts_per_page'];
+				$sql_limit = $attachments_per_page;
 
 				if ($start > $num_files / 2)
 				{
 					$store_reverse = true;
 
-					if ($start + $config['posts_per_page'] > $num_files)
+					if ($start + $attachments_per_page > $num_files)
 					{
-						$sql_limit = min($config['posts_per_page'], max(1, $num_files - $start));
+						$sql_limit = min($attachments_per_page, max(1, $num_files - $start));
 					}
 
 					// Select the sort order. Add time sort anchor for non-time sorting cases
@@ -1145,32 +1178,36 @@ class acp_attachments
 				$result = $db->sql_query_limit($sql, $sql_limit, $sql_start);
 
 				$i = ($store_reverse) ? $sql_limit - 1 : 0;
+
+				// Store increment value in a variable to save some conditional calls
+				$i_increment = ($store_reverse) ? -1 : 1;
 				while ($attachment_row = $db->sql_fetchrow($result))
 				{
 					$attachments_list[$i] = $attachment_row;
-					($store_reverse) ? $i-- : $i++;
+					$i = $i + $i_increment;
 				}
 				$db->sql_freeresult($result);
 
 				$template->assign_vars(array(
 					'TOTAL_FILES'		=> $num_files,
-					'TOTAL_SIZE'		=> $total_size,
+					'TOTAL_SIZE'		=> get_formatted_filesize($total_size),
-					'PAGINATION'		=> generate_pagination($this->u_action . "&amp;$u_sort_param", $num_files, $config['posts_per_page'], $start, true),
+					'PAGINATION'		=> generate_pagination($this->u_action . "&amp;$u_sort_param", $num_files, $attachments_per_page, $start, true),
 
-					'S_ON_PAGE'			=> on_page($num_files, $config['posts_per_page'], $start),
+					'S_ON_PAGE'			=> on_page($num_files, $attachments_per_page, $start),
 					'S_LIMIT_DAYS'		=> $s_limit_days,
 					'S_SORT_KEY'		=> $s_sort_key,
 					'S_SORT_DIR'		=> $s_sort_dir)
 				);
 
-				// Grab extensions information
+				// Grab extensions
 				$extensions = $cache->obtain_attach_extensions(true);
 
 				for ($i = 0, $end = sizeof($attachments_list); $i < $end; ++$i)
 				{
-					$row =& $attachments_list[$i];
+					$row = $attachments_list[$i];
 
 					$row['extension'] = strtolower(trim((string) $row['extension']));
+					$comment = ($row['attach_comment']) ? str_replace(array("\n", "\r"), array('<br />', "\n"), $row['attach_comment']) : '';
 					$display_cat = $extensions[$row['extension']]['display_cat'];
 					$l_downloaded_viewed = ($display_cat == ATTACHMENT_CATEGORY_NONE) ? 'DOWNLOAD_COUNT' : 'VIEWED_COUNT';
 					$l_download_count = (!isset($row['download_count']) || (int) $row['download_count'] == 0) ? $user->lang[$l_downloaded_viewed . '_NONE'] : (((int) $row['download_count'] == 1) ? sprintf($user->lang[$l_downloaded_viewed], $row['download_count']) : sprintf($user->lang[$l_downloaded_viewed . 'S'], $row['download_count']));
@@ -1181,8 +1218,8 @@ class acp_attachments
 						'FILETIME'			=> $user->format_date((int) $row['filetime']),
 						'REAL_FILENAME'		=> utf8_wordwrap(utf8_basename((string) $row['real_filename']), 40, '<br />', true),
 						'PHYSICAL_FILENAME'	=> utf8_basename((string) $row['physical_filename']),
+						'COMMENT'			=> $comment,
 						'TOPIC_TITLE'		=> (!$row['in_message']) ? (string) $row['topic_title'] : '',
-						'DISABLED'			=> 'disabled="disabled"',
 						'ATTACH_ID'			=> (int) $row['attach_id'],
 						'POST_ID'			=> (int) $row['post_msg_id'],
 						'TOPIC_ID'			=> (int) $row['topic_id'],

phpBB/includes/acp/info/acp_attachments.php

@@ -24,7 +24,7 @@ class acp_attachments_info
 				'extensions'	=> array('title' => 'ACP_MANAGE_EXTENSIONS', 'auth' => 'acl_a_attach', 'cat' => array('ACP_ATTACHMENTS')),
 				'ext_groups'	=> array('title' => 'ACP_EXTENSION_GROUPS', 'auth' => 'acl_a_attach', 'cat' => array('ACP_ATTACHMENTS')),
 				'orphan'		=> array('title' => 'ACP_ORPHAN_ATTACHMENTS', 'auth' => 'acl_a_attach', 'cat' => array('ACP_ATTACHMENTS')),
-				'attachments'	=> array('title' => 'ACP_MANAGE_ATTACHMENTS', 'auth' => 'acl_a_attach', 'cat' => array('ACP_ATTACHMENTS')),
+				'manage'		=> array('title' => 'ACP_MANAGE_ATTACHMENTS', 'auth' => 'acl_a_attach', 'cat' => array('ACP_ATTACHMENTS')),
 			),
 		);
 	}

phpBB/install/database_update.php

@@ -1958,6 +1958,13 @@ function change_database_data(&$no_updates, $version)
 					'auth'		=> 'acl_a_group',
 					'cat'		=> 'ACP_GROUPS',
 				),
+				'manage'	=> array(
+					'base'		=> 'attachments',
+					'class'		=> 'acp',
+					'title'		=> 'ACP_MANAGE_ATTACHMENTS',
+					'auth'		=> 'acl_a_attach',
+					'cat'		=> 'ACP_ATTACHMENTS',
+				),
 			);
 
 			_add_modules($modules_to_install);

phpBB/language/en/acp/common.php

@@ -118,7 +118,7 @@ $lang = array_merge($lang, array(
 	'ACP_MAIN'					=> 'ACP index',
 
 	'ACP_MANAGE_ATTACHMENTS'			=> 'Manage attachments',
-	'ACP_MANAGE_ATTACHMENTS_EXPLAIN'	=> 'Here you can manage files attached to posts. You are able to delete the files or reassign them to existing posts. Reassigning to posts requires a valid post ID, you have to determine this ID by yourself. This will reassign the already assigned attachment to the post you entered.',
+	'ACP_MANAGE_ATTACHMENTS_EXPLAIN'	=> 'Here you can list and delete files attached to posts and private messages.',
 
 	'ACP_MANAGE_EXTENSIONS'		=> 'Manage extensions',
 	'ACP_MANAGE_FORUMS'			=> 'Manage forums',
@@ -238,6 +238,9 @@ $lang = array_merge($lang, array(
 	'EXPORT_DOWNLOAD'		=> 'Download',
 	'EXPORT_STORE'			=> 'Store',
 
+	'FILES_GONE'			=> 'Some of the attachments you selected for deletion do not exist. They may have been already deleted. Attachments that did exist were deleted.',
+	'FILES_STAT_WRONG'		=> 'Your files statistics is probably inaccurate and might need to be resynchronised. Actual values: number of attachments » %1$d, total size of attachments » %2$s.',
+
 	'GENERAL_OPTIONS'		=> 'General options',
 	'GENERAL_SETTINGS'		=> 'General settings',
 	'GLOBAL_MASK'			=> 'Global permission mask',
@@ -263,6 +266,7 @@ $lang = array_merge($lang, array(
 	'NOTIFY'				=> 'Notification',
 	'NO_ADMIN'				=> 'You are not authorised to administer this board.',
 	'NO_EMAILS_DEFINED'		=> 'No valid e-mail addresses found.',
+	'NO_FILES_TO_DELETE'	=> 'Attachments you selected for deletion do not exist.',
 	'NO_PASSWORD_SUPPLIED'	=> 'You need to enter your password to access the Administration Control Panel.',
 
 	'OFF'					=> 'Off',