? ? ? Pending

User tests: Successful: Unsuccessful:

avatar richard67
richard67
16 May 2021

Pull Request for Issue # .

Summary of Changes

This pull request (PR) adds missing assets and corrects wrong lft and rgt values in the #__assets table for new installations.

The following mistakes are corrected:

  1. PR #25516 has removed asset (ID 13) for com_mailto but hasn't updated lft and rgt.
  2. PR #25570 has added 2 new modules (ID's 107 and 108) which use new asset (ID's 84 and 85), but these assets haven't been added.
  3. The same PR has added 3 new modules (ID's 96 to 98) which use the same assets (ID's 41, 42 and 45) as already existing modules (ID's 3,4 and 10), but they should use their own assets.
  4. PR #33550 has removed asset (ID 69) for com_csp but hasn't updated lft and rgt.

At least for the last PR it's partly my fault because I had reviewed the SQL parts of it.

But I had planned to check that anyway and provide the necessary fix, what I do now with this PR here.

Testing Instructions

Requirements

Testers need to know and have understood the principles of hierarchical tables using the nested set model as described here:

https://en.wikipedia.org/wiki/Nested_set_model

The #__assets table uses an extended model which saves as redundant information also parent_id and level in addition to lft and rgt for speeding up certain queries.

My following drawing shows the same model for the #__usergroups table and might help to understand this model.

usergroup-tree-calculation

I just see one line missing between the "Editor" and "Publisher", but I think you get it.

The values in the box are the ID's, the number left beside the box are lftand right beside the box are rgt, and the arrows show how the values are incremented.

If you select data from such a table and sort it by lft, you can easily check if there are errors in lft and rgt values if the table is not too big and has not too many levels in hierarchy. For the #__assets table it is still possible for me.

Instructions

Step 1: Copy the CREATE TABLE statement for the #__assets table and the INSERT statements for this table from the base.sql file for your database type from the current 4.0-dev branch without this PR into the SQL command window of an SQL client tool.

E.g. if using MySQL (or MariaDB), copy the code from here into phpMyAdmin's SQL command tab:
https://github.com/joomla/joomla-cms/blob/4.0-dev/installation/sql/mysql/base.sql#L8-L103

Don't replace the #__ table prefix so it doesn't have impact on any existing tables.

Execute the SQL so the table is created and all records are inserted.

Step 2: Execute following SQL command to get a list or the assets sorted by lft:

SELECT `id`,`parent_id`,`lft`,`rgt`,`level`,`name` FROM `#__assets` ORDER BY `lft`;

Make sure that all records are shown (checkbox "Show all" in phpMyAdmin).

Step 3: Check if the lft and rgt values are correct with respect to the model described above and the parent_id and level values.

Result: There are errors, details see section "Actual result BEFORE applying this Pull Request" below.

Step 4: Export the result of this query (all records) into an SQL file.

Step 5: Delete all records from the table.

Step 6: Copy the INSERT statements for the #__assets table from the base.sql file of this PR for your database type into the SQL command window of an SQL client tool.

E.g. if using MySQL (or MariaDB), copy the code from here into phpMyAdmin's SQL command tab:

INSERT INTO `#__assets` (`id`, `parent_id`, `lft`, `rgt`, `level`, `name`, `title`, `rules`) VALUES
(1, 0, 0, 161, 0, 'root.1', 'Root Asset', '{"core.login.site":{"6":1,"2":1},"core.login.admin":{"6":1},"core.login.api":{"8":1},"core.login.offline":{"6":1},"core.admin":{"8":1},"core.manage":{"7":1},"core.create":{"6":1,"3":1},"core.delete":{"6":1},"core.edit":{"6":1,"4":1},"core.edit.state":{"6":1,"5":1},"core.edit.own":{"6":1,"3":1}}'),
(2, 1, 1, 2, 1, 'com_admin', 'com_admin', '{}'),
(3, 1, 3, 6, 1, 'com_banners', 'com_banners', '{"core.admin":{"7":1},"core.manage":{"6":1}}'),
(4, 1, 7, 8, 1, 'com_cache', 'com_cache', '{"core.admin":{"7":1},"core.manage":{"7":1}}'),
(5, 1, 9, 10, 1, 'com_checkin', 'com_checkin', '{"core.admin":{"7":1},"core.manage":{"7":1}}'),
(6, 1, 11, 12, 1, 'com_config', 'com_config', '{}'),
(7, 1, 13, 16, 1, 'com_contact', 'com_contact', '{"core.admin":{"7":1},"core.manage":{"6":1}}'),
(8, 1, 17, 38, 1, 'com_content', 'com_content', '{"core.admin":{"7":1},"core.manage":{"6":1},"core.create":{"3":1},"core.edit":{"4":1},"core.edit.state":{"5":1},"core.execute.transition":{"6":1,"5":1}}'),
(9, 1, 39, 40, 1, 'com_cpanel', 'com_cpanel', '{}'),
(10, 1, 41, 42, 1, 'com_installer', 'com_installer', '{"core.manage":{"7":0},"core.delete":{"7":0},"core.edit.state":{"7":0}}'),
(11, 1, 43, 44, 1, 'com_languages', 'com_languages', '{"core.admin":{"7":1}}'),
(12, 1, 45, 46, 1, 'com_login', 'com_login', '{}'),
(14, 1, 47, 48, 1, 'com_massmail', 'com_massmail', '{}'),
(15, 1, 49, 50, 1, 'com_media', 'com_media', '{"core.admin":{"7":1},"core.manage":{"6":1},"core.create":{"3":1},"core.delete":{"5":1}}'),
(16, 1, 51, 54, 1, 'com_menus', 'com_menus', '{"core.admin":{"7":1}}'),
(17, 1, 55, 56, 1, 'com_messages', 'com_messages', '{"core.admin":{"7":1},"core.manage":{"7":1}}'),
(18, 1, 57, 128, 1, 'com_modules', 'com_modules', '{"core.admin":{"7":1}}'),
(19, 1, 129, 132, 1, 'com_newsfeeds', 'com_newsfeeds', '{"core.admin":{"7":1},"core.manage":{"6":1}}'),
(20, 1, 133, 134, 1, 'com_plugins', 'com_plugins', '{"core.admin":{"7":1}}'),
(21, 1, 135, 136, 1, 'com_redirect', 'com_redirect', '{"core.admin":{"7":1}}'),
(23, 1, 137, 138, 1, 'com_templates', 'com_templates', '{"core.admin":{"7":1}}'),
(24, 1, 143, 146, 1, 'com_users', 'com_users', '{"core.admin":{"7":1}}'),
(26, 1, 147, 148, 1, 'com_wrapper', 'com_wrapper', '{}'),
(27, 8, 18, 19, 2, 'com_content.category.2', 'Uncategorised', '{}'),
(28, 3, 4, 5, 2, 'com_banners.category.3', 'Uncategorised', '{}'),
(29, 7, 14, 15, 2, 'com_contact.category.4', 'Uncategorised', '{}'),
(30, 19, 130, 131, 2, 'com_newsfeeds.category.5', 'Uncategorised', '{}'),
(32, 24, 144, 145, 2, 'com_users.category.7', 'Uncategorised', '{}'),
(33, 1, 149, 150, 1, 'com_finder', 'com_finder', '{"core.admin":{"7":1},"core.manage":{"6":1}}'),
(34, 1, 151, 152, 1, 'com_joomlaupdate', 'com_joomlaupdate', '{}'),
(35, 1, 153, 154, 1, 'com_tags', 'com_tags', '{}'),
(36, 1, 155, 156, 1, 'com_contenthistory', 'com_contenthistory', '{}'),
(37, 1, 157, 158, 1, 'com_ajax', 'com_ajax', '{}'),
(38, 1, 159, 160, 1, 'com_postinstall', 'com_postinstall', '{}'),
(39, 18, 58, 59, 2, 'com_modules.module.1', 'Main Menu', '{}'),
(40, 18, 60, 61, 2, 'com_modules.module.2', 'Login', '{}'),
(41, 18, 62, 63, 2, 'com_modules.module.3', 'Popular Articles', '{}'),
(42, 18, 64, 65, 2, 'com_modules.module.4', 'Recently Added Articles', '{}'),
(43, 18, 66, 67, 2, 'com_modules.module.8', 'Toolbar', '{}'),
(44, 18, 68, 69, 2, 'com_modules.module.9', 'Notifications', '{}'),
(45, 18, 70, 71, 2, 'com_modules.module.10', 'Logged-in Users', '{}'),
(46, 18, 72, 73, 2, 'com_modules.module.12', 'Admin Menu', '{}'),
(48, 18, 78, 79, 2, 'com_modules.module.14', 'User Status', '{}'),
(49, 18, 80, 81, 2, 'com_modules.module.15', 'Title', '{}'),
(50, 18, 82, 83, 2, 'com_modules.module.16', 'Login Form', '{}'),
(51, 18, 84, 85, 2, 'com_modules.module.17', 'Breadcrumbs', '{}'),
(52, 18, 86, 87, 2, 'com_modules.module.79', 'Multilanguage status', '{}'),
(53, 18, 90, 91, 2, 'com_modules.module.86', 'Joomla Version', '{}'),
(54, 16, 52, 53, 2, 'com_menus.menu.1', 'Main Menu', '{}'),
(55, 18, 94, 95, 2, 'com_modules.module.87', 'Sample Data', '{}'),
(56, 8, 20, 37, 2, 'com_content.workflow.1', 'COM_WORKFLOW_BASIC_WORKFLOW', '{}'),
(57, 56, 21, 22, 3, 'com_content.state.1', 'COM_WORKFLOW_BASIC_STAGE', '{}'),
(58, 56, 23, 24, 3, 'com_content.transition.1', 'Publish', '{}'),
(59, 56, 25, 26, 3, 'com_content.transition.2', 'Unpublish', '{}'),
(60, 56, 27, 28, 3, 'com_content.transition.3', 'Archive', '{}'),
(61, 56, 29, 30, 3, 'com_content.transition.4', 'Trash', '{}'),
(62, 56, 31, 32, 3, 'com_content.transition.5', 'Feature', '{}'),
(63, 56, 33, 34, 3, 'com_content.transition.6', 'Unfeature', '{}'),
(64, 56, 35, 36, 3, 'com_content.transition.7', 'Publish & Feature', '{}'),
(65, 1, 139, 140, 1, 'com_privacy', 'com_privacy', '{}'),
(66, 1, 141, 142, 1, 'com_actionlogs', 'com_actionlogs', '{}'),
(67, 18, 74, 75, 2, 'com_modules.module.88', 'Latest Actions', '{}'),
(68, 18, 76, 77, 2, 'com_modules.module.89', 'Privacy Dashboard', '{}'),
(70, 18, 88, 89, 2, 'com_modules.module.103', 'Site', '{}'),
(71, 18, 92, 93, 2, 'com_modules.module.104', 'System', '{}'),
(72, 18, 96, 97, 2, 'com_modules.module.91', 'System Dashboard', '{}'),
(73, 18, 98, 99, 2, 'com_modules.module.92', 'Content Dashboard', '{}'),
(74, 18, 100, 101, 2, 'com_modules.module.93', 'Menus Dashboard', '{}'),
(75, 18, 102, 103, 2, 'com_modules.module.94', 'Components Dashboard', '{}'),
(76, 18, 104, 105, 2, 'com_modules.module.95', 'Users Dashboard', '{}'),
(77, 18, 106, 107, 2, 'com_modules.module.99', 'Frontend Link', '{}'),
(78, 18, 108, 109, 2, 'com_modules.module.100', 'Messages', '{}'),
(79, 18, 110, 111, 2, 'com_modules.module.101', 'Post Install Messages', '{}'),
(80, 18, 112, 113, 2, 'com_modules.module.102', 'User Status', '{}'),
(82, 18, 114, 115, 2, 'com_modules.module.105', '3rd Party', '{}'),
(83, 18, 116, 117, 2, 'com_modules.module.106', 'Help Dashboard', '{}'),
(84, 18, 118, 119, 2, 'com_modules.module.107', 'Privacy Requests', '{}'),
(85, 18, 120, 121, 2, 'com_modules.module.108', 'Privacy Status', '{}'),
(86, 18, 122, 123, 2, 'com_modules.module.96', 'Popular Articles', '{}'),
(87, 18, 124, 125, 2, 'com_modules.module.97', 'Recently Added Articles', '{}'),
(88, 18, 126, 127, 2, 'com_modules.module.98', 'Logged-in Users', '{}');

Don't replace the #__ table prefix.

Execute the SQL so that all records are inserted.

Step 7: Execute the same SQL command as in step 2 to get a list or the assets sorted by lft.

Make sure that all records are shown (checkbox "Show all" in phpMyAdmin).

Step 8: Check if the lft and rgt values are correct with respect to the model described above and the parent_id and level values.

Result: The lft and rgt values in the #__assets are correct.

Step 9: Export the result of this query (all records) into an SQL file, using a different file name than in step 4.

Step 10: Compare the 2 files exported in steps 4 and 9 using a good comparison tool like e.g. Beyond Compare, the one in TotalCommander, Araxis Merge or if you have an IDE what that ships.

Result: The comparison shows only changes in lft and rgt values and the 5 added records (2 missing and 3 for the wrong duplicate uses) due to PR #25570 .

The ordering of names and the values of id, parent_id and level of previously existing records haven't changed.

Step 11: Using the same tool as in the previous step, compare the INSERT statements for the #__assets table in the 2 files modified by this PR, to make sure they are the same for both database types.

This shall make sure that the changes in the file for the other database type are equal to the changes you have tested in the previous steps for your database type.

Result: The INSERT statements for the #__assets table in the 2 files modified by this PR have the same data.

Step 12: Check that the system tests in drone have passed for all database types.

Result: The system tests in drone have passed, so this PR hasn't made any SQL syntax errors in the base.sql for new installations.

Step 13 - optional test for paranoid testers who think I might have broken something:

Apply the patch of this PR to a clean, current 4.0-dev or latest 4.0 nightly and make a new installation into an empty database.

Check that everything related to assets, i.e. permissions, works at least as well as before.

Actual result BEFORE applying this Pull Request

  • Gap between rgt 46 and lft 49 (asset ID's 12 and 14) caused by PR #25516 .
  • Duplicate lft and rgt values 74 and 75 for asset ID's 46 and 67 caused by PR #25570 .
  • Gap between rgt 79 and lft 82 (asset ID's 48 and 49) caused by PR #25570 .
  • Duplicate lft and rgt values 90 and 91 for asset ID's 53 and 70 caused by PR #25570 .
  • Duplicate lft and rgt values 92 and 93 for asset ID's 55 and 71 caused by PR #25570 .
  • Gap between rgt 126 and lft 129 (asset ID's 21 and 65).
  • Duplicate lft and rgt values 129 and 130 for asset ID's 23 and 65.
  • Duplicate lft value 131 for asset ID's 24 and 66 .
  • Wrong lft and rgt values 132 and 133 for asset ID 24 due to the previous error.

2021-05-16_j4-asset-table_errors

Expected result AFTER applying this Pull Request

  • The lft and rgt values in the #__assets are correct:
    2021-05-16_j4-asset-table_corrected

  • The comparison between the old and the corrected data ordered by lft shows only changes in lft and rgt values and the 5 added records (2 missing and 3 for the wrong duplicate uses) due to PR #25570 . The ordering of names, the ID's and the parent ID's haven't changed. See the following comparisons with left side = data with changes from this PR and right side = data without these changes as it is now in 4.0-dev.

2021-05-16_j4-asset-table_diff-1

2021-05-16_j4-asset-table_diff-2

Additional information

I've checked lft and rgt values of all other nested tables where we add records in installation SQL scripts, and those were ok.

The same for asset_id values: Beside the wrong duplicate use corrected by this PR here, I have found no other mistakes or missing assets.

Documentation Changes Required

None.

avatar richard67 richard67 - open - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 16 May 2021
Category SQL Installation Postgresql
avatar richard67 richard67 - change - 16 May 2021
The description was changed
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
The description was changed
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
The description was changed
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
The description was changed
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
The description was changed
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
The description was changed
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
The description was changed
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
The description was changed
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
The description was changed
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
The description was changed
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
The description was changed
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
The description was changed
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
The description was changed
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
The description was changed
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
Labels Added: ?
avatar richard67
richard67 - comment - 16 May 2021

Setting the release blocker label as we should not ship new installs with corrupted data.

avatar richard67 richard67 - change - 16 May 2021
Title
[4.0] [WiP] Fix assets table lft and rgt and add missing assets for modules
[4.0] Fix assets table lft and rgt and add missing assets for modules
avatar richard67 richard67 - change - 16 May 2021
Title
[4.0] Fix assets table lft and rgt and add missing assets for modules
[4.0] [WiP] Fix assets table lft and rgt and add missing assets for modules
avatar richard67 richard67 - edited - 16 May 2021
avatar richard67 richard67 - change - 16 May 2021
Title
[4.0] [WiP] Fix assets table lft and rgt and add missing assets for modules
[4.0] Fix assets table lft and rgt and add missing assets for modules
avatar ceford
ceford - comment - 18 May 2021

I went through this as far as Test 10 at which stage I ran out of brains. It all looks good to me. Thanks for excellent explanation.


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

avatar richard67
richard67 - comment - 18 May 2021

I went through this as far as Test 10 at which stage I ran out of brains. It all looks good to me. Thanks for excellent explanation.

@ceford So it wasn't a good test because not complete? If so, how can I help to complete it?

avatar ceford
ceford - comment - 18 May 2021

I can't do Step 12 because I don't know anything about drone. Or does that mean check that the drone check on github has passed? I could probably do Step 13.


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

avatar richard67
richard67 - comment - 18 May 2021

@ceford The drone thing means only to check that "All checks have passed" is shown with green check mark at the bottom of the PR on GitHub.

avatar richard67
richard67 - comment - 18 May 2021

If I had SQL syntax error in the base.sql, it would show that some tests have failed.

avatar ceford ceford - test_item - 18 May 2021 - Tested successfully
avatar ceford
ceford - comment - 18 May 2021

I have tested this item successfully on d07104e

I have completed as far as and including Step 13 and everything seems to work. I have a nodding acquaintance with Nested Sets and everything I looked at seemed OK. I am content to pass this PR. But, I can't think of a sensible test on a new install that would show an error if something were amiss.


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

avatar alikon alikon - test_item - 18 May 2021 - Tested successfully
avatar alikon
alikon - comment - 18 May 2021

I have tested this item successfully on d07104e

code review


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

avatar alikon alikon - change - 18 May 2021
Status Pending Ready to Commit
avatar alikon
alikon - comment - 18 May 2021

RTC


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

avatar wilsonge wilsonge - close - 19 May 2021
avatar wilsonge wilsonge - merge - 19 May 2021
avatar wilsonge wilsonge - change - 19 May 2021
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-05-19 21:55:42
Closed_By wilsonge
Labels Added: ? ?
avatar wilsonge
wilsonge - comment - 19 May 2021

Thanks!

avatar richard67
richard67 - comment - 19 May 2021

Thanks all!

Add a Comment

Login with GitHub to post a comment