Skip to content

Conversation

@dbenham
Copy link
Contributor

@dbenham dbenham commented Feb 23, 2023

Fix is not perfect, the code in question needs refactoring.

However, the most common use case that was seeing a frequent error now works.

@dbenham dbenham requested a review from nkissebe February 23, 2023 20:25
@nkissebe
Copy link
Contributor

Can you describe what the issue is and what you did to fix it. I don't see any problem code wise, just want to know what is being fixed.

@dbenham
Copy link
Contributor Author

dbenham commented Feb 27, 2023

I disliked making either of these changes, because the failures were not some obscure edge case that was overlooked, it was pretty basic stuff that wasn't working. And why did it work before (assuming it did) and not now? I don't have a good answer for that.

@dbenham
Copy link
Contributor Author

dbenham commented Mar 20, 2023

Reasoning for changes

core/components/com_installer/admin/controllers/customexts.php

Change at line 628

The custom extension page said the componet was up to date. This was an error, when there are changes upstream, $fetch_respoonse was non empty, and it's contents had to be displayed in order to show the changes to the user and enable the 'merge' button on the form to allow the user to merge the changes. This addition accomplished this.

Change at line 687

Originally this code just did a preg_grep, but a successful update could return an empty $update_response, and that resulted in a php error that $update_response was undefined. I added a check to prevent this, plus, I added a check for a completely blank update_response to indicate a successful update as well.

Incidentally, I never saw the case where $update_response was non empty and the regular expression check for "updating the repository" returned true, but I left that seeing no need to remove it.

core/components/com_installer/admin/views/customexts/tmpl/fetched.php

Change at line 34

Code simply wasn’t needed, it resulted in a double display of the success message.

@dbenham dbenham merged commit ad3549c into dev Mar 20, 2023
@nkissebe nkissebe deleted the custom-extensions-bug branch April 18, 2024 22:46
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.

3 participants