Skip to content
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

[5.2] Error handling on extension update #43321

Merged
merged 11 commits into from
Nov 7, 2024
Merged

Conversation

BrainforgeUK
Copy link
Contributor

Pull Request for Issue #43319 .

Summary of Changes

Catch errors from the extension being updated and abort update with a sensible message.

Testing Instructions

(a) Installed this Plugin.
plg_updatetest1.zip

(b) Enable the plugin.

(c) Go to Joomla updater and check for updates.

(d) Then select the plugin installed above PLG_HIKASHOPSHIPPING_BFEXTRAFEATURES and update it.

Actual result BEFORE applying this Pull Request

Error 0

Expected result AFTER applying this Pull Request

Error 1

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@brianteeman
Copy link
Contributor

Cannot be tested
image

@BrainforgeUK
Copy link
Contributor Author

Try this test plugin - screenshots before applying the pull request changes.
plg_updatetest2.zip

Error 10
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
Error 15
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
Error 12
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
Error 16

@brianteeman
Copy link
Contributor

image

@BrainforgeUK
Copy link
Contributor Author

Test does not work on Unix system! - must be filename case, investigating.

@brianteeman
Copy link
Contributor

I am using windows

@BrainforgeUK
Copy link
Contributor Author

Sorted, no change to the above needed, can go ahead with testing.
Appologizes for inconvenience, update server issue, not looked at that for a long time!

@brianteeman
Copy link
Contributor

With Debug Disabled
image

With Debug Enabled
image

PLEASE check your work before submitting it

@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Apr 20, 2024
@BrainforgeUK
Copy link
Contributor Author

Changed.

Sorry, copied the changed code across and forgot the use statement.
Failed to check the files changed count - thought I had copied the language text across.

@brianteeman
Copy link
Contributor

You also forgot to check the language string you used and the string you created.

@BrainforgeUK
Copy link
Contributor Author

Retested as: #43321 (comment)

With changes made the message is now:

restest2

@brianteeman
Copy link
Contributor

I am all in favour of meaningfull error messages but they also have to be helpful. In this example the advice is not helpful at all as it means nothing at all to the user. This is a message that is only useful to the original extension developer and is not something that an end user shoudl ever see. Extension developers dont release updates without testing they work do they

@BrainforgeUK
Copy link
Contributor Author

Regardless of who is to blame for the error what I propose is better and more useful than what Joomla currently displays.

This is actually not a plugin bug - the plugin is used by and depends on a 3rd party extension, it is that dependency which causes the problem when the update is attempted with the plugin enabled. Could modify such plugins to protect against such errors but I see that as unnecessary code in the plugin when Joomla can handle it more gracefully.

I suppose could add another line to the error message:
"Please contact the extension developer."

Could enhance further by adding contact information from current XML?

@brianteeman
Copy link
Contributor

The question is. Does the new message help the end user more than the original message. For me the answer is no

@BrainforgeUK
Copy link
Contributor Author

The end user is an admin who hopefully knows the extensions installed on their site.

The current error is missleading as it implies it is a bug in the Joomla installer ... which led to me being asked a question which started me on this.

@HLeithner HLeithner added the bug label Apr 24, 2024
@HLeithner HLeithner changed the title Error handling on extension update [5.1] Error handling on extension update Apr 24, 2024
@chmst
Copy link
Contributor

chmst commented May 9, 2024

I think that this helps "Please contact the extension developer of the Plugin ... ".
Then I know that it is not a bug in Joomla or in my confirguration and don't waste time with debugging.

@brianteeman
Copy link
Contributor

@chmst the problem is that according to @BrainforgeUK the problem is not with the developer of this plugin but with the developer of a completely different extension upon which it depends - so which extension developer am I going to contact? For me that is not the problem. The problem is that the extension that is being installed is not checking for the presence of the extension it is dependent upon. ergo nothing sensible can be changed in core to fix an extension developers errors

@chmst
Copy link
Contributor

chmst commented May 9, 2024

@brianteeman - thanks, agreed.

@TLWebdesign
Copy link

On Joomla 5.1.3 i'm not seeing the path. I tested it both on a localhost and on a live test server.
See attached screenshot:
Scherm­afbeelding 2024-08-24 om 12 45 57

@brianteeman
Copy link
Contributor

@TLWebdesign as this PR is for 5.1 and not 5.2 then you should be marking this as an unsuccessful test

@TLWebdesign
Copy link

I have tested this item 🔴 unsuccessfully on bb1c6aa

On 5.1.3 i'm not getting the path to the file to show.


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

@HLeithner HLeithner changed the base branch from 5.1-dev to 5.2-dev September 2, 2024 08:27
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.2-dev.

@TLWebdesign
Copy link

I have tested this item ✅ successfully on bb1c6aa

Tested Succesfully on 5.2


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

@HLeithner HLeithner changed the title [5.1] Error handling on extension update [5.2] Error handling on extension update Sep 2, 2024
@Quy
Copy link
Contributor

Quy commented Oct 28, 2024

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 28, 2024
@pe7er pe7er self-assigned this Nov 2, 2024
@pe7er pe7er enabled auto-merge (squash) November 7, 2024 08:35
@pe7er pe7er merged commit 83333d4 into joomla:5.2-dev Nov 7, 2024
3 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 7, 2024
@pe7er
Copy link
Contributor

pe7er commented Nov 7, 2024

Thanks @BrainforgeUK !

@Hackwar Hackwar added this to the Joomla! 5.2.1 milestone Nov 7, 2024
Kostelano added a commit to JPathRu/localisation that referenced this pull request Nov 27, 2024
joomla/joomla-cms#44139 +
joomla/joomla-cms#44286 +
joomla/joomla-cms#44166 +
joomla/joomla-cms#43321 +
joomla/joomla-cms#44266 - (только для др. пакетов)
joomla/joomla-cms#44452 - (только для др. пакетов)
+ испр. ошибки #118
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.