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 lft
and 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.