[#11060] - JRoute between clients (site -> admin | admin -> site)
- Closed
- 27 Jun 2017
- Medium
- Build: staging
- # 11060
- Diff
- andrepereiradasilva:jroute-between-sites
- Success continuous-integration/drone the build was successful Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
User tests: Successful: Unsuccessful:
Pull Request for Improvement.
Summary of Changes
This PR is for allowing JRoute to generate URL's from admin to site and vice-versa.
This is a current problem especially if you are trying to make links from the backend to the frontend.
So, with this PR, there is no need anymore to make special code just to get the frontend link from the backend. With this PR we just need to use JRoute, like all other links.
The same will work to links from frontend to backend.
After this PR, no need to hardcode frontend to admin links.
Testing Instructions
Test 1: Links from admin to site
- Use latest staging with sample data from at least two languages (en-GB, fr-FR)
- Add to isis index.php file...
$app = JFactory::getApplication();
$app->enqueueMessage('1. Admin to Admin: ' . JRoute::_('index.php'), 'notice');
$app->enqueueMessage('2. Admin to Admin: ' . JRoute::_('index.php?option=com_content'), 'notice');
$app->enqueueMessage('3. Admin to Site: ' . JRoute::_('index.php?option=com_content', true, null, 'site'), 'notice');
$app->enqueueMessage('4. Admin to Site (MutilLanguage 1): ' . JRoute::_('index.php?option=com_content&lang=fr', true, null, 'site'), 'notice');
$app->enqueueMessage('5. Admin to Site (MutilLanguage 2): ' . JRoute::_('index.php?option=com_users&lang=en', true, null, 'site'), 'notice');- Open any admin page. Check the URL in resulting notice message are correct.
Note: You should test also other URLs (articles, menu items, etc).
Note 2: Try with the several variation of SEF/non-SEF, with or without URL rewritting, with or without multilanguage (i.e., with or without language filter plugin enabled).
Test 2: Links from site to admin
Add to protostar index.php file...
$app = JFactory::getApplication();
$app->enqueueMessage('1. Site to Site: ' . JRoute::_('index.php'), 'notice');
$app->enqueueMessage('2. Site to Site: ' . JRoute::_('index.php?option=com_content'), 'notice');
$app->enqueueMessage('3. Site to Admin: ' . JRoute::_('index.php?option=com_content', true, null, 'administrator'), 'notice');- Open any frontend page. Check the URL in resulting notice message are correct.
Note: You should test also other URLs.
Note 2: Try with the several variation of SEF/non-SEF, with or without URL rewritting, with or without multilanguage (i.e., with or without language filter plugin enabled).
Test 3: Code review
Do a code review.
Test 4: Current URLs
Check if current URL generated by JRouter are exactly the same, i.e., no change in current URL.
Probably the best is to leave the patch applied for some time, while you do other things and check there are no problems with the current URLs.
Observations
Thanks to @mbabker, @ggppdk and @infograf768 for helping out with this one.
Mantainers, to make sure that are no unintended consequences, it would be good if this can be also reviewed from someone that understand the Router better than i do.
| Status | New | ⇒ | Pending |
| Labels |
Added:
?
|
||
| Category | ⇒ | Libraries Router / SEF |
I did a quick code review and I really like the implementation. Thanks for implementing this!
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11060.
can anyone force a travis rebuild here?
Restarted travis as requested
thanks @brianteeman
same failure ...
PHP Fatal error: Cannot instantiate abstract class JSessionStorage in /home/travis/build/joomla/joomla-cms/libraries/joomla/session/storage.php on line 85
i don't understand why this happens.
can anyone help with this?
First thought is that your branch is not upto date (thats been the problem
in the past)
On 10 July 2016 at 10:49, andrepereiradasilva notifications@github.com
wrote:
thansk @brianteeman https://github.com/brianteeman
same failure ...
PHP Fatal error: Cannot instantiate abstract class JSessionStorage in
/home/travis/build/joomla/joomla-cms/libraries/joomla/session/storage.php
on line 85i don't understand with this happens.
can anyone help with this?—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11060 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABPH8T9Pvky4UuIKsbnKeQOUlDCM97zAks5qUMAUgaJpZM4JIl1C
.
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
well but it is up to date, so is not that.
looking at the error call stack https://travis-ci.org/joomla/joomla-cms/jobs/143540387#L962 it seems to be related with JRoute in opensearch ... ?
Now you are beyond where I can help - sorry
Maybe @mbabker or @wilsonge have an idea
On 10 July 2016 at 10:53, andrepereiradasilva notifications@github.com
wrote:
well but it is up to date, so is not that.
looking at the error call stack
https://travis-ci.org/joomla/joomla-cms/jobs/143540387#L962 it seems to
be related with JRoute in opensearch ... ?—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11060 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABPH8R636lcRvXVyq_nvKY9X6niYHJNLks5qUMEYgaJpZM4JIl1C
.
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
@andrepereiradasilva The thing is by default with JApplicationCms::getInstance($client) as you know you create a new session (except for cli), which is mocked in Unit tests.
Somehow the session handler (fallback 'none') became an empty String '' after you changes.
$handler = $conf->get('session_handler', 'none');
Which causes that it tries to instantiate the abstract class JSessionStorage. (There is no isAbstract() / isInstantiable() test before).
@yvesh thanks
but i didn't change any session related stuff as you can see from the code changes https://github.com/joomla/joomla-cms/pull/11060/files
update: forget it i see what you mean now.
This PR has received new commits.
CC: @Fedik
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11060.
On a first quick look across the code, it looks pretty good, however I see one issue with backwards compatibility. Changing JRoute::$_router from an object to an array has in the past been defined as a breach in backwards compatibility.
I fear a few issues with problematic component routers, but I will have to take a deeper look into this first.
Huh.. How can private variable cause backwards compatibility break as it cannot be accessed from anywhere else but this class? It cannot be used even from child classes and therefore it cannot break anything.
Of course you can use reflection, but... I think the point of having private variable is that it cannot be accessed and is internal to the specific class only.
Sorry, you are right. I missed that it was private and not just protected.
The correct tests for multilanguage to add in index.php should be
$app->enqueueMessage('4. Admin to Site (MutilLanguage 1): ' . JRoute::_('index.php?option=com_content&lang=fr-FR', true, null, 'site'), 'notice');
$app->enqueueMessage('5. Admin to Site (MutilLanguage 2): ' . JRoute::_('index.php?option=com_users&lang=en-GB', true, null, 'site'), 'notice');
The reason is that someone may use en-US and en-GB, or fr-FR and fr-CA, etc.
Test successful here.
I have tested this item
Great!
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11060.
See example of use here:
#10932 (comment)
I have tested and it seems it works for front-end with SEF and URL Rewriting but for back-end URL(ADMIN to SITE) remains same. Is that correct?
See front-end:
See Back-end:
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11060.
I have tested and it seems it works for front-end with SEF and URL Rewriting but for back-end URL(ADMIN to SITE) remains same. Is that correct?
@hardiktailored no is not correct. you should also have SEF URL from admin to site
can you tell which options did you use for global config SEF? if you are using multilanguage and if you have the language filter plugin enabled.
| Milestone |
Added: |
||
administrator has no SEF URLs.
@andrepereiradasilva Yes, exactly. I think SITE URLs should also be rewrite when SEF and Rewriting enabled even when linking from ADMIN.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11060.
and it should be, that's why i asked you those questions:
can you tell which options did you use for global config SEF? if you are using multilanguage and if you have the language filter plugin enabled.
I have language filter plugin disabled so ignore 4th and 5th URL but don't you think 3rd URL should be changed?
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11060.
with that config, with or without language filter plugin enabled, all 3 should be changed.
strange i can't reproduce your issue
are you sure you have the patch applied?
Enabling Language Filter plugin, URL rewrites. But that should also work without need of this plugin enabled.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11060.
Yes, patch applied.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11060.
ok. will check again
you're right, somehow it doesn't work now without the language filter plugin enabled
also i discovered another issue when you have an installed language that is default language for site that does not have a content language and language plugin enabled we get PHP notices
Notice: Undefined index: pt-BR in /path/to/joomla-staging/plugins/system/languagefilter/languagefilter.php on line 190
Notice: Trying to get property of non-object in /path/to/joomla-staging/plugins/system/languagefilter/languagefilter.php on line 190
Notice: Undefined index: pt-BR in /path/to/joomla-staging/plugins/system/languagefilter/languagefilter.php on line 190
Notice: Trying to get property of non-object in /path/to/joomla-staging/plugins/system/languagefilter/languagefilter.php on line 190
so clearly more improvements are needed
This PR has received new commits.
CC: @Fedik, @infograf768
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11060.
hm, it seems stopped to work, or I just confused
I tried test on sample data (in backend):
JRoute::_('index.php?option=com_content&view=article&id=17', true, null, 'site')and expected to see: /article-category-blog/17-first-blog-post
but got /index.php?option=com_content&view=article&id=17&Itemid=435, without SEF, and with wrong Itemid.
Maybe because some recent router changes?
Sorry, I don't know and unfortunately also don't have the time to investigate this.
Ok.thanks anyway
rdeutz
- | Milestone |
Added: |
||
| Milestone |
Removed: |
||
This PR needs a new owner or it will have to be closed as abandoned.
I'd like to take the charge and little rework, if acceptable?
Take it
No it's absolutely fine :) Take it!
closing as @izharaazmi took over this
see #16879
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-06-27 11:03:14 |
| Closed_By | ⇒ | andrepereiradasilva |
| Category | Libraries Router / SEF | ⇒ | Libraries Front End Plugins Router / SEF |
travis error:
? nothing to do with this PR i think