-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds error if target db has the same name as current db #13099 #13104
Conversation
Signed-off-by: TheSaturn <saturn@thesaturn.me>
Codecov Report
@@ Coverage Diff @@
## master #13104 +/- ##
==========================================
- Coverage 54.35% 54.35% -0.01%
==========================================
Files 466 466
Lines 69612 69582 -30
==========================================
- Hits 37841 37818 -23
+ Misses 31771 31764 -7 Continue to review full report at Codecov.
|
@@ -48,6 +48,8 @@ | |||
|
|||
if (! isset($_REQUEST['newname']) || strlen($_REQUEST['newname']) === 0) { | |||
$message = PMA\libraries\Message::error(__('The database name is empty!')); | |||
} else if($_REQUEST['newname'] === $_REQUEST['db']) { | |||
$message = PMA\libraries\Message::error(__('The new database name is equal to the current one!')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest changing this message to "Cannot copy database to the same name. Change the name and try again."
@@ -27,7 +27,7 @@ | |||
<?php endif; ?> | |||
<td class="small_tab" | |||
title="<?= __('Show/hide columns'); ?>" | |||
id="id_hide_tbody_<?= $t_n_url; ?>" | |||
id="id_hide_tbody_<?= htmlspecialchars($t_n_url) ?>" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should really split unrelated changes in to different pull requests, even though both of these changes are only one or two lines each.
I have a database named p&?&
and this doesn't seem to fix the problem for that or with the database name in the original error report, either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've cherry picked this as 4119b5c.
Also note that there is #13103 which is working towards changing the defaults for the the copy/rename fields. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As written by Isaac - try to submit one pull request with one fix. That way it's easier to merge parts of it when they are ready to get merged...
I've committed dbdd7bc based on your fix and @ibennetch wording. |
Adds error if target db has the same name as current db name to prevent data corrupt #13099