? ? Pending

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
22 Apr 2021

Pull Request for Issue ##33173.

Summary of Changes

Adds an additional message which contains the link to the templates component

Testing Instructions

I honestly have no idea

cb16ded 22 Apr 2021 avatar brianteeman .
avatar brianteeman brianteeman - open - 22 Apr 2021
avatar brianteeman brianteeman - change - 22 Apr 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 22 Apr 2021
Category Administration Language & Strings Front End Plugins
avatar particthistle
particthistle - comment - 23 Apr 2021

Yes, not an easy core one to test. Need to create a test component to install with an override to trigger something... or find a need to modify a template item in Cassiopeia.

@Anu1601CS do you have any test components that you had floating around from when you created the override check and comparison functionality?

avatar PhilETaylor
PhilETaylor - comment - 23 Apr 2021

Might want to update the PR title as "first commit" doesnt really explain this :)

avatar brianteeman brianteeman - change - 23 Apr 2021
Labels Added: ? ?
avatar brianteeman brianteeman - change - 23 Apr 2021
Title
first commit
[4.0] On updating Joomla, "Overridden files have changed" message should be linked to allow resolution.
avatar brianteeman brianteeman - edited - 23 Apr 2021
590f64a 23 Apr 2021 avatar brianteeman enter
avatar richard67
richard67 - comment - 23 Apr 2021

Regarding testing: How about using patchtester? Install version 4.0.0, make some override for it, and then update to 4.0.1?

avatar brianteeman
brianteeman - comment - 23 Apr 2021

@richard67 Might work. I was trying to test it with core last night but you cant because the file was replaced in the core update. Will test again myself later today

avatar richard67
richard67 - comment - 23 Apr 2021

Regarding testing: How about using patchtester? Install version 4.0.0, make some override for it, and then update to 4.0.1?

Hmm, no, doesn't work. No overrides can be made for the patchtester. Have to find another one.

avatar richard67
richard67 - comment - 23 Apr 2021

2021-04-23_1

My review suggestion above fixes that.

avatar richard67
richard67 - comment - 23 Apr 2021

@brianteeman I've just tested with a 3rd party admin module. Both the link added with this PR as well as the Quickicon for the overrides check point to site templates, but they should point to admin templates in case of an admin module.

Do you think this can be fixed for your link with this PR here, and then for the Quickicon with another PR?

Or should I ignore that and give this PR a good test result?

avatar richard67
richard67 - comment - 23 Apr 2021

For testing I have used https://www.phoca.cz/download/category/123-phoca-top-menu-module . There is a glitch with the Override Check not related to this PR: I did not get the alert modified by this PR when updating that module from 4.0.4. to 4.0.5. So I have uninstalled that extension after I had created the override, and then installed it again, and then I could see that alert.

avatar brianteeman
brianteeman - comment - 23 Apr 2021

I would say thats unrelated. but a genuine issue with the override check

avatar richard67
richard67 - comment - 23 Apr 2021

I would say thats unrelated. but a genuine issue with the override check

@brianteeman Does that refer to my last comment about the glitch? Then I agree with you. Or does it refer to my comment before that one about link always going to site templates, also for admin modules? In this case I have to think about it.

avatar richard67
richard67 - comment - 23 Apr 2021

In the #__template_overrides table in database is information about the template, so it should be possible to check if it's a site or an admin template. But maybe that should be done with another, future PR, at all places, including the one being added here.

avatar brianteeman
brianteeman - comment - 23 Apr 2021

The one about the site templates. If the quickicon also goes there (and I suspect it does) then its a bigger bug than addressed here

avatar brianteeman
brianteeman - comment - 23 Apr 2021

If this PR goes to the same url as the quickicon then this PR is correct and anything else should be its own issue

avatar richard67 richard67 - test_item - 23 Apr 2021 - Tested successfully
avatar richard67
richard67 - comment - 23 Apr 2021

I have tested this item successfully on 320fa88

Tested with use of https://www.phoca.cz/download/category/123-phoca-top-menu-module .

There is a glitch with the Override Check not related to this PR: I did not get the alert modified by this PR when updating that module from 4.0.4. to 4.0.5. So I have uninstalled that extension after I had created the override, and then installed it again, and then I could see that alert.

The link in the alert added by this PR has the same target as the Quickicon for the Override Checks has. Both have the problem pointing to site templates in any case, also if the override was made in an admin template e.g. for an admin module.

So this PR is consistent with the Quickicon.

The other issues mentioned above need to be fixed separately.


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

avatar richard67 richard67 - test_item - 23 Apr 2021 - Not tested
avatar richard67
richard67 - comment - 23 Apr 2021

I have not tested this item.

Sorry, my previous test was wrong.

The quickicon correctly points to admin templates if the override was made in an admin template, and so should do the link added by this PR.


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

avatar richard67
richard67 - comment - 23 Apr 2021

Hmm, now after having logged out and in again, the Quickicon points to site templates, which is wrong again.

avatar richard67
richard67 - comment - 23 Apr 2021

The more I think about it, the less I know what's the right way. What if one has overrides in site and in admin templates, where should that link go to?

I think this PR is ok so far as it solve the issue it refers to and is an improvement, and other glitches have to be solved separately.

avatar richard67 richard67 - test_item - 23 Apr 2021 - Tested successfully
avatar richard67
richard67 - comment - 23 Apr 2021

I have tested this item successfully on 320fa88


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

avatar richard67
richard67 - comment - 24 Apr 2021

It seems that both the link added here and the quickicon go to either admin or site templates depending on which of these you have visited the last time before in your session, i.e. if you once have been in the admin templates view and now use the link or the quickicon, you get to the admin templates, but after logout and login again it goes to site templates. I have some vague memory that this was subject of some issue or some comment in some issue or PR, but I am not sure. Maybe someone else remembers better if that is a known issue?

avatar brianteeman
brianteeman - comment - 24 Apr 2021

It was for editing modules

avatar Quy
Quy - comment - 7 Jul 2021

33256

avatar brianteeman
brianteeman - comment - 7 Jul 2021

What is the output when language debug is NOT enabled?

avatar Quy
Quy - comment - 7 Jul 2021

33256-no-debug

avatar brianteeman
brianteeman - comment - 13 Jul 2021

Closing as the more I look at it i dont think this is the correct approach

avatar brianteeman brianteeman - change - 13 Jul 2021
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2021-07-13 06:29:34
Closed_By brianteeman
avatar brianteeman brianteeman - close - 13 Jul 2021

Add a Comment

Login with GitHub to post a comment