User tests: Successful: Unsuccessful:
PR for #17377
There is a comment in the original code that makes zero sense and the code with it is the cause odf the problem. This PR removes that code
Before this PR if you create a bulk import it inserts the site name see video
Category | ⇒ | Administration com_redirect |
Status | New | ⇒ | Pending |
@brianteeman Can you try replacing the section of code within the loop and before the query with:
$root = JUri::root();
// Source URLs need to have the correct URL format to work properly
if (strpos($batch_url[0], $root) === false)
{
$old_url = str_replace($root . '/', $root, $root . $batch_url[0]);
}
else
{
$old_url = $batch_url[0];
}
// Destination URL can also be an external URL
if (!empty($batch_url[1]) && strpos('http', $batch_url[1]) !== false)
{
$new_url = $batch_url[1];
}
elseif (!empty($batch_url[1]))
{
$new_url = str_replace($root . '/', $root, $root . $batch_url[1]);
}
else {
$new_url = '';
}
this should insert the urls correctly and remove the double slash.
@tonypartridge the issue is that it is different in the bulk import to the manual entry
@brianteeman it is, we also need to fix that. since we cannot use relative urls in Joomla! com_redirects it has to be absolute. Relative links do not work. I'm looking at that now, ideally relative links would be better.
@brianteeman ok I'm going to do a bigger pull shortly which some good improvements to the com_redirects.
@tonypartridge are you aware of #17653?
@franz-wohlkoenig yep I already commented on it ;-)
@tonypartridge it was an attempt at a quick fix to make the behaviour on inserting a record the same when you do a bulk import as a manual import
Yep, but actually it's broken. Inserting as relative isn't support by
com_redirects for installations in a sub folder hence a bigger pull is
needed
--
Tony Partridge
From: Brian Teeman notifications@github.com notifications@github.com
Reply: joomla/joomla-cms
reply@reply.github.com
reply@reply.github.com
Date: 22 August 2017 at 07:46:06
To: joomla/joomla-cms joomla-cms@noreply.github.com
joomla-cms@noreply.github.com
CC: Tony Partridge admin@xtech.im admin@xtech.im, Mention
mention@noreply.github.com mention@noreply.github.com
Subject: Re: [joomla/joomla-cms] [com_redirect] Bulk import (#17658)
@tonypartridge https://github.com/tonypartridge it was an attempt at a
quick fix to make the behaviour on inserting a record the same when you do
a bulk import as a manual import—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#17658 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABVglmU2mCNaSbyJRa6YnUNFZInrV3zgks5sankugaJpZM4O9lPt
.
OK well I will close this then and await your PR instead.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-08-22 06:48:58 |
Closed_By | ⇒ | brianteeman | |
Labels |
Added:
?
|
Actually to me it makes sense, since it only inserts the url if the it doesn't contain the root url and the redirects component can only act on the root url.
Doing what you are doing actually will break the importer for anyone who is just inserting the relative path already.
I think we just need to add the same method to the new url, which is actually just a http/https test.
The issue with @coolcat-creations is she is inserting a / before, to solve her issue I would just use:
to
Will fix her issue, but we also have bigger issues like, bulk import allows importing already existing urls and invalid redirect new urls.