-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Remove deprecated code for administrator #12278
Conversation
Please add the SQL Update files to remove the module |
@zero-24 you mean com_admin/sql ? |
Yes :) |
As well as the normal install sql under installation/sql |
@zero-24 ok, done. Travis happy (at least not complaining about these changes) |
@dgt41 i mean the extension ;) As you removed the mod_submenu extension we need to run on update a delete on the extension table and in the install sql we need to remove that extension too :) |
👍 |
Documentation Changes RequiredRemoved extension & removed methods |
@@ -0,0 +1,3 @@ | |||
DELETE FROM `#__assets` WHERE `id` = 47; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be removed from the update SQL. We cannot guarantee asset ID 47 is this module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we purge the assets structure anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK we don't have any updates that touch the assets table with direct SQL (aside from schema changes). It's too risky making changes to its contents for a wide variety of reasons, which is in part why there's that method in the update script to check the base assets for new components are added.
@@ -0,0 +1,3 @@ | |||
DELETE FROM `#__assets` WHERE `id` = 47; | |||
|
|||
DELETE FROM `#__extensions` WHERE `name` = ''mod_submenu''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The quoting on this line is busted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm this looks not merged see: https://github.com/joomla/joomla-cms/pull/12281/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only fixed the quoting with 2bc5a9a - not the other part :)
Pull Request for Issue # .
Summary of Changes
Testing Instructions
Admin should still work
Documentation Changes Required