Allow moving attachments for multiple projects #964

Merged
merged 3 commits into from Dec 29, 2016

Projects

None yet

3 participants

@jllano
Contributor
jllano commented Nov 29, 2016
  • replaced radio button with checkbox
  • minor layout adjustments
  • refactor logic in action page to make sure multiple project selection works

Fixes #21927

Before:

screen shot 2016-11-29 at 9 21 20 pm
After:

after

@vboctor

I would change the PR title and commit title to "Allow moving attachments for multiple projects".

admin/move_attachments.php
+$t_moved = array();
+
+if ( null != $f_project_to_move ) {
+ foreach ( $f_project_to_move as $t_project_to_move) {
@vboctor
vboctor Nov 29, 2016 Member

Remove space before ( and add a space after $t_project_to_move and before ).

admin/move_attachments.php
+ $t_array = explode( ':', $t_project_to_move );
+
+ if( isset( $t_array[1] ) ) {
+ $f_project_id = $t_array[1];
@vboctor
vboctor Nov 29, 2016 Member

$f_field is only for form fields and not for temporary / calculated fields. Please change to $t_project_id.

admin/move_attachments.php
@@ -291,9 +295,13 @@ function move_attachments_to_disk( $p_type, array $p_projects ) {
# Display results
if( empty( $t_moved ) ) {
- echo '<p class="lead">Nothing to do.</p>'. "\n";
+ echo '<div class="alert alert-danger">';
+ echo '<p class="lead"><strong>Opps!</strong> Please select the project you want to move the attachment.</p>';
@vboctor
vboctor Nov 29, 2016 Member

The old message indicated that there is no work to do and didn't mention anything about projects not selected. Which can be the case of there were projects selected that had no attachments to be moved. The new one seems to indicate that there is no project selected. If we want the message to indicate that a project is selected, then we should look at project move list, otherwise, I would keep the old message to cover the case where there are no projects selected or projects selected, but no work to be done.

admin/move_attachments.php
@@ -343,6 +351,7 @@ function move_attachments_to_disk( $p_type, array $p_projects ) {
echo '</div>';
echo '</div>';
echo '</div>';
+ echo "<br/>";
@vboctor
vboctor Nov 29, 2016 Member

By default use single quotes unless double quotes are needed.

admin/move_attachments.php
@@ -343,6 +351,7 @@ function move_attachments_to_disk( $p_type, array $p_projects ) {
echo '</div>';
echo '</div>';
echo '</div>';
+ echo "<br/>";
}
}
echo "<br/>";
@vboctor
vboctor Nov 29, 2016 Member

Use single quote unless double quotes are needed.

@jllano jllano Allow moving attachments for multiple projects
- replaced radio button with checkbox
- minor layout adjustments
- refactor logic in action page to make sure multiple project selection works

Fixes #21927
8eaba18
@jllano jllano changed the title from Allow user to move multiple attachment to Allow moving attachments for multiple projects Nov 30, 2016
@jllano jllano Improve efficiency of processing
- show message if there is no project selected
- keep old message to cover the case only if projects is selected (optimized)
- fixed coding standards

Fixes #21927
0711f71
@jllano
Contributor
jllano commented Nov 30, 2016

@vboctor I already addressed your comments above.

@dregad

Looking at the screenshots (did not actually test), the tables' column headers' styling is not consistent with the rest of the application.

I think we should use the same style for tables throughout MantisBT.

@jllano
Contributor
jllano commented Dec 3, 2016

@dregad I already addressed your concern.

@dregad
dregad approved these changes Dec 5, 2016 View changes
@vboctor vboctor merged commit 068cfa6 into mantisbt:master Dec 29, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment