-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[4.0] Fix assets table lft and rgt and add missing assets for modules #33924
[4.0] Fix assets table lft and rgt and add missing assets for modules #33924
Conversation
Setting the release blocker label as we should not ship new installs with corrupted data. |
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. |
@ceford So it wasn't a good test because not complete? If so, how can I help to complete it? |
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. |
@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. |
If I had SQL syntax error in the base.sql, it would show that some tests have failed. |
I have tested this item ✅ successfully on d07104e This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33924. |
I have tested this item ✅ successfully on d07104e This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33924. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33924. |
Thanks! |
Thanks all! |
Pull Request for Issue # .
Summary of Changes
This pull request (PR) adds missing assets and corrects wrong
lft
andrgt
values in the#__assets
table for new installations.The following mistakes are corrected:
com_mailto
but hasn't updatedlft
andrgt
.com_csp
but hasn't updatedlft
andrgt
.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 alsoparent_id
andlevel
in addition tolft
andrgt
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 arergt
, 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 inlft
andrgt
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 theINSERT
statements for this table from thebase.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
:Make sure that all records are shown (checkbox "Show all" in phpMyAdmin).
Step 3: Check if the
lft
andrgt
values are correct with respect to the model described above and theparent_id
andlevel
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 thebase.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
andrgt
values are correct with respect to the model described above and theparent_id
andlevel
values.Result: The
lft
andrgt
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
andrgt
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
andlevel
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
Expected result AFTER applying this Pull Request
The

lft
andrgt
values in the#__assets
are correct:The comparison between the old and the corrected data ordered by
lft
shows only changes inlft
andrgt
values and the 5 added records (2 missing and 3 for the wrong duplicate uses) due to PR [4.0] Backend template #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.Additional information
I've checked
lft
andrgt
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.