Skip to content

Fix tags single instance view - #28746

Merged
HLeithner merged 1 commit into
joomla:stagingfrom
wilsonge:fix/tags-single-view
Apr 21, 2020
Merged

Fix tags single instance view#28746
HLeithner merged 1 commit into
joomla:stagingfrom
wilsonge:fix/tags-single-view

Conversation

@wilsonge

Copy link
Copy Markdown
Contributor

Summary of Changes

Single tags view creates a 404 when viewing a tag. Introduced in #28610

Whilst hit doesn't need to load the article hasPrimaryKey a few lines later does.

Testing Instructions

View a single tag in the frontend. Before the patch tag view 404's. afterwards doesn't.

Documentation Changes Required

None

@zero-24 zero-24 added this to the Joomla! 3.9.18 milestone Apr 21, 2020
@richard67

Copy link
Copy Markdown
Member

I have tested this item ✅ successfully on 6c3f8a0


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

@HLeithner
HLeithner merged commit 36ff3ba into joomla:staging Apr 21, 2020
@HLeithner

Copy link
Copy Markdown
Member

Thanks

@wilsonge
wilsonge deleted the fix/tags-single-view branch April 21, 2020 17:29
@bayareajenn

Copy link
Copy Markdown

I have tested this item ✅ successfully on 6c3f8a0

Resolves issue with both menu item for single tag and with module tags - popular.


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

@peteruoi

Copy link
Copy Markdown

Is the new release gonna published soon or should we patch the code?

@bayareajenn

Copy link
Copy Markdown

Is the new release gonna published soon or should we patch the code?

See https://forum.joomla.org/viewtopic.php?p=3602147#p3602150 a new release will be published very soon.

@peteruoi

Copy link
Copy Markdown

Thanks a lot!

@Septdir

Septdir commented Apr 21, 2020

Copy link
Copy Markdown
Contributor

It might be more correct to remove this check.
I can’t understand why it is needed here.
https://github.com/joomla/joomla-cms/blob/staging/components/com_tags/models/tag.php#L348

@Septdir

Septdir commented Apr 21, 2020

Copy link
Copy Markdown
Contributor

It turns out that this is a partial rollback #28610
I don’t think it’s right.

@zero-24

zero-24 commented Apr 21, 2020

Copy link
Copy Markdown
Contributor

@Septdir this fixes an breaking issue. Feel free to propose an PR for 3.9.18 to get it right. In oder to get the patch out fast we decided to partly revert that mention PR.

@Septdir

Septdir commented Apr 21, 2020

Copy link
Copy Markdown
Contributor

@zero-24 I could have done it, but it’s already late in the evening and we won’t start making it.
And yes, the release has already been released.

Here you can discuss the need for this 404.
In order not to do another pr just like that.

@zero-24

zero-24 commented Apr 21, 2020

Copy link
Copy Markdown
Contributor

Sorry mixed up the versions. I was about to write 3.9.19 should the PR be targeted at.

I could have done it, but it’s already late in the evening and we won’t start making it.
And yes, the release has already been released.

Agree it is late here too and we took extra hours to make sure we get the release out very fast.

Here you can discuss the need for this 404.
In order not to do another pr just like that.

As said feel free to do the PR tomorrow on in the next days 3.9.19 should hopefully take some more time than 3.9.18 ;)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants