wrong links to news detail (view)

[Jimmi08]

Bug Description

On my site, I noticed that news URLs are created the weird way in news listing in any news list except rss plugin.

How to Reproduce

My settings:

RSS plugin: (correct)
https://www.e107sk.com/news/view/62/jm-core-1-9-released

News:
https://www.e107sk.com/news/view/62/plugins/jm-core-1-9-released

there is the category added to URL.

News template uses:
<h3 class="post-title">{NEWS_TITLE: link=1}</h3>
and
<a href="{NEWSURL}" class="pull-right">' . LAN_READ_MORE . ' »</a>

both shortcodes return wrong URL.

RSS plugin uses:
$rss[$i]['link'] = e107::getUrl()->create('news/view/item', $value, 'full=1');

And
news shortcodes
e107::getUrl()->create('news/view/item', $this->news_item);

RSS works because it doesn't have category_sef in $value.

It means that e107::getUrl() ignores URL configuration.

PHP 7.2
latest git version for testing, the live site is older.

Thanks

[CaMer0n]

@Jimmi08 Are you aware of the RSS URL format being used anywhere else in e107?

[Jimmi08]

@Jimmi08 Are you aware of the RSS URL format being used anywhere else in e107?

No idea. Maybe in some of my plugins, but the format is the same, and the RSS version is correct.

[CaMer0n]

@Jimmi08 Actually, the sef example was wrong and the RSS format was incorrect.

[Jimmi08]

@CaMer0n Why did you (generally) do this change? And when?

[Jimmi08]

Admin view is the next example where is the category missing in URL...

You can delete category without checking if news exists...

e107 has already problems with double content, now it is worse.

[Jimmi08]

e_gsitemap generates links without category too

[Jimmi08]

More than a year ago it looked like this:

Maybe a month ago it looks like this:

(something was changed because the order is changed)

Now it looks like this (correctly), so only now I am able to tell that you changed something so important.

You changed not only URL structure but the number of URL parts.
Can you let me where was this change made? What commit or issue? Thanks

[Deltik]

@Jimmi08: The change in behavior from your second screenshot to your third screenshot was made in 74a3735. See git bisect here:

www-data@e107-dev:~/html/e107$ git bisect start
www-data@e107-dev:~/html/e107$ git bisect new
www-data@e107-dev:~/html/e107$ git checkout a04db4e2c8204a166f2e3e152d04eb0f55a719e1
Note: switching to 'a04db4e2c8204a166f2e3e152d04eb0f55a719e1'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at a04db4e2c Hotfix: Don't fatal error if core_image.php is corrupt
www-data@e107-dev:~/html/e107$ git bisect old
Bisecting: 49 revisions left to test after this (roughly 6 steps)
[e4de8502c5474609cf7391fba1081b211197ac68] Fixes #3926, #4135 - support for attributes onchange, onclick etc. when script access is enabled.
www-data@e107-dev:~/html/e107$ git bisect old
Bisecting: 24 revisions left to test after this (roughly 5 steps)
[8c254bccd6f4cccdeb95034a8fd73fddf424ef46] Issue #4146 - Fixed user profile navigation (next/prev) for legacy, bootstrap3 and bootstrap4 themes. Closes #4021 - Added support for independent core templates for legacy, bootstrap3 and bootstrap4 templates.
www-data@e107-dev:~/html/e107$ git bisect old
Bisecting: 12 revisions left to test after this (roughly 4 steps)
[e86d6272faffb338abc1ef246e0cf64f1897f0a3] Private Messenger: Bootstrap 4 styling fixes.
www-data@e107-dev:~/html/e107$ git bisect old
Bisecting: 6 revisions left to test after this (roughly 3 steps)
[7081737f12470dfe4d0d677524780821f9291294] Fixes #4165 - Custom field problem with single quote value.
www-data@e107-dev:~/html/e107$ git bisect old
Bisecting: 3 revisions left to test after this (roughly 2 steps)
[41056cb12a3aa4b6bf1da25737c2368f765a0f76] Improved e_date::computerLapse() test.
www-data@e107-dev:~/html/e107$ git bisect old
Bisecting: 1 revision left to test after this (roughly 1 step)
[74a3735488becc44394d4c207b6198a78baf91a9] Fixes #4172 - Incorrect RSS URLs on news and incorrect SEF URL example.
www-data@e107-dev:~/html/e107$ git bisect new
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[80c5bd7582ad95d13a7438999dffa7e255d42abf] Fixes #4171 - checkbox visibility issue.
www-data@e107-dev:~/html/e107$ git bisect old
74a3735488becc44394d4c207b6198a78baf91a9 is the first new commit
commit 74a3735488becc44394d4c207b6198a78baf91a9
Author: Cameron <e107inc@gmail.com>
Date:   Wed May 20 11:49:05 2020 -0700

    Fixes #4172 - Incorrect RSS URLs on news and incorrect SEF URL example.

 e107_core/url/news/sef_url.php   |  2 +-
 e107_plugins/news/e_rss.php      |  2 +-
 e107_plugins/rss_menu/e_meta.php | 41 +++++++++++++++++++++-------------------
 3 files changed, 24 insertions(+), 21 deletions(-)

I was not able to find a change that turned your first screenshot into your second screenshot. On master, the one you've highlighted still shows up as the third one for me:

[Jimmi08]

@Deltik I know, this was the latest change when I noticed that and it was fix for RSS plugin. But sometimes before was added the category to SEF-URL version. I checked that core/news/sef_url.php wasn't changed, it looks like category_sef wasn't sent before, so it probably hit another route. When I did my canonical plugin, I really checked this and I would notice something like this.

Nevermind, at least I had to look at how this works and I was able to change it to my liking.

I don't have a problem with the category in URL, I have a problem with that next slash. And that you (generally) did change again in just part of the system, not everywhere.

But again, I have it fixed now, all canonical URLs I have generated correct way now and if I update core and I forget to change this file again, it would not be the problem, because canonical URLs are now correct. So it is good that this happened.

[CaMer0n]

@Jimmi08 I didn't change the underlying processing of the URL. I simply updated the example URL (which is hard-coded) and did not match the reality of what the URL profile generates.

As you saw, the file has barely changed since 2016:
https://github.com/e107inc/e107/commits/master/e107_core/url/news/sef_url.php

So it is good that this happened.

Indeed, it is, so we removed the inconsistency. Another solution is to create a new 'news' URL profile which does NOT have the category included. If you want to do that, I'll be happy to add it to the core for you.

[CaMer0n]

@Jimmi08 If you find other instances of the category-sef missing, please leave a message about it here. Thanks.

[Jimmi08]

@CaMer0n I know that you didn't change anything now. That this was just fixing. It is ok.

When you don't send category_sef to url(), it will display that old way. So sometimes ago something changed in news data.

The problem is not in a category (it is good in fact), the problem is in that slash. And missing mandatory slash at end of URL.

My fix is: - changed slash to underscore.

'View/<id:{number}>/<category:{sefsecure}>_<name:{sefsecure}>'
'urlSuffix' => '.html',