Skip to content

[4.0] Fix JS errors for com_joomlaupdate#33488

Merged
wilsonge merged 12 commits intojoomla:4.0-devfrom
joomdonation:fix_js_com_joomlaupdate
May 6, 2021
Merged

[4.0] Fix JS errors for com_joomlaupdate#33488
wilsonge merged 12 commits intojoomla:4.0-devfrom
joomdonation:fix_js_com_joomlaupdate

Conversation

@joomdonation
Copy link
Copy Markdown
Contributor

Pull Request for Issue # .

Summary of Changes

There are currently many js errors in com_joomlaupdate. This PR attempts to fix this.

Testing Instructions

I'm unsure as I don't understand fully how pre-update checks in com_joomlaupdate works yet. Maybe for now, code review would be enough because the current code is already broken.

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels May 2, 2021
@richard67
Copy link
Copy Markdown
Member

@joomdonation It needs to fix javascript-cs, see https://ci.joomla.org/joomla/joomla-cms/42929/1/21 .

@richard67 richard67 mentioned this pull request May 2, 2021
@joomdonation
Copy link
Copy Markdown
Contributor Author

@wilsonge As of right now, all the errors are fixed and I would say that it works in the same way with what we are having on 3.10.0. You can review the code and get it merged (I made basic test by installing older version of Akeeba Backup and JCE editor for testing)

However, there are still some problems:

Basically, from what I understand, the loop over all the none core plugins and if that plugin belong to an extension, remove it from the array. But that could cause all plugins load being checked (causes by the removal mentioned). Maybe you can ask the developer who worked on the feature to take a look at it.

@dgrammatiko
Copy link
Copy Markdown
Contributor

However, there are still some problems:

George had asked me to fix this after the BS js PR but I never made a PR because my solution (SPA basically)was touching the restore.php and that wasn't a discussion I wanted myself involved. This is (I think) the repo: https://github.com/joomla/joomla-cms/compare/4.0-dev...dgrammatiko:4.0-dev_joomlaUpdate?expand=1

At the very least check the changes in the controller and the way I pass data from PHP to JS. The current state is god awful...

@joomdonation
Copy link
Copy Markdown
Contributor Author

@dgrammatiko I haven't spent time to read the whole code to understand the process, so I could not comment on your solution right now. However, if you make a PR, I would be happy to review and test it. The current code is not looking good, I agree.

@wilsonge
Copy link
Copy Markdown
Contributor

wilsonge commented May 3, 2021

Starting to look better. Thanks for fixing my mess trying to port this!

@wilsonge wilsonge merged commit 2c1fbe7 into joomla:4.0-dev May 6, 2021
@wilsonge
Copy link
Copy Markdown
Contributor

wilsonge commented May 6, 2021

This is clearly an improvement merging this now

@wilsonge wilsonge added this to the Joomla 4.0 milestone May 6, 2021
@joomdonation joomdonation deleted the fix_js_com_joomlaupdate branch May 6, 2021 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants