User tests: Successful: Unsuccessful:
Pull Request for Issue # .
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:
com_mailto but hasn't updated lft and rgt.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.
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.
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.
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:
joomla-cms/installation/sql/mysql/base.sql
Lines 27 to 108 in a5c03e3
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.
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.
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.
None.
| Status | New | ⇒ | Pending |
| Category | ⇒ | SQL Installation Postgresql |
| Labels |
Added:
?
|
||
| Title |
|
||||||
| Title |
|
||||||
| Title |
|
||||||
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.
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.
If I had SQL syntax error in the base.sql, it would show that some tests have failed.
I have tested this item
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.
I have tested this item
code review
| Status | Pending | ⇒ | Ready to Commit |
RTC
| 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:
?
?
|
||
Thanks!
Thanks all!
Setting the release blocker label as we should not ship new installs with corrupted data.