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

Fixing URLs for orphaned articles #14031

Merged
merged 1 commit into from Feb 12, 2017

Conversation

Projects
None yet
7 participants
@Hackwar
Member

Hackwar commented Feb 11, 2017

Pull Request for Issue #13978 .

Summary of Changes

The issue is, that the home menu item is of type article and thus all orphaned articles are thought as being the homepage article. This is again because we are checking the path of views and even check the last view in the path. (the one without any further children) However, if we are at that point, we should be skipping that view, since we should either have matched that in the first step, where we match for the exactly matching menu item or not find a match at all.

Testing Instructions

Please see the instructions in #13978

Expected result

Actual result

Documentation Changes Required

@infograf768

This comment has been minimized.

Member

infograf768 commented Feb 12, 2017

Tested;
From test instructions:

How to reproduce the bugs

Try navigating to article 2
Note that the article cannot be accessed.

This one is solved by this PR.
BUT trying to to navigate to article 3 loads the category list menu item

Go to the category list view you create and go into article 3
Go to article 2
Note you go back to to the category 2 view and not article 2.

That one is not solved: it loads the category list menu item
modernrouter2

@wilsonge

This comment has been minimized.

Contributor

wilsonge commented Feb 12, 2017

This also works for me for the orphaned articles. I'm going to merge this with my and JM's test but the second half of the issue with the navigation from the category view needs fixing as he mentioned still so I will leave the original issue open for that

@wilsonge wilsonge merged commit 14bfccf into joomla:staging Feb 12, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@wilsonge wilsonge added this to the Joomla 3.7.0 milestone Feb 12, 2017

@bayareajenn

This comment has been minimized.

bayareajenn commented Feb 12, 2017

From Article 1 I clicked the Article 2 link and it takes me to Article 2 properly.
From Article 2 I clicked on Article 3 and it took me to Article 3 properly.
From Article 3 I clicked on Article 2 and it took me the the category list with Article 3 in it.

So I agree and can confirm what JM and George got too.

PS I have no idea what I'm doing with PRs. I manually replaced the code in the file in here with what was in the site via FTP and of course missed a { because I lack knowledge but eventually I got it! Was that the right way to do it? Sorry for cluttering this up with me not being quite clever enough to know what I don't know. ;)

@wilsonge

This comment has been minimized.

Contributor

wilsonge commented Feb 13, 2017

Applying manually works. It's probably easier for you to use patchtester tho Jenn - you can find the instructions here :) https://docs.joomla.org/Testing_Joomla!_patches

@bayareajenn

This comment has been minimized.

bayareajenn commented Feb 13, 2017

Ah, yes, the patchtester! I forgot about it. Thanks, George.

@JoshHouser

This comment has been minimized.

JoshHouser commented Mar 24, 2017

Does this still need testing?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14031.

@rdeutz

This comment has been minimized.

Contributor

rdeutz commented Mar 24, 2017

@JoshuaLewis not at this moment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment