-
-
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
Fixing #__assets table #30178
Fixing #__assets table #30178
Conversation
Please add the missing information. |
Added the PostgreSQL changes and testing instruction |
Do we need explicit asset entries for all components - I'm only asking because we never seem to have had entries before. So unsure why we need this now. Obviously removing the non-existing component is fine |
@Formatio-hippocampi that is probably because the changes in this PR are in the installation sql files and you have probably mistakenly deleted the installation folder |
Or not mistakenly deleted because it is not a Git clone but a "normal" installation, where the folder has to be removed at the end. But that's the reason, definitely. The installation folder is not there so the Patchtester can't apply changes in the installation SQL script. |
It does not have to be removed with a beta |
@fastslack could you solve the conflicts? |
Is this still relevant? Looks to me like it was addressed by @richard67 in #33924 and can be closed? |
Well the asset for the mass mail component was not removed with my PR. |
sorry - i confused mailto and massmail |
btw this has become obsolete in the meantime as for an example com_csp don't exist |
This pull request has automatically rebased to 4.2-dev. |
while trying to solve the conflicts I made a list of row we might need to have, positing it here so that it don't get lost: com_actionlogs |
While trying to solve the conflicts I found a problem. The problem with this PR is that it adds some of the missing assets to the top, e.g. 'com_workflow' with id = 2 and so on, so that other assets move down, e.g. 'com_admin' moved from id = 2 to id = 7. But the asset IDs of existing modules are not changed. This cannot be right, and there was no change in the mean time in the base branch which could have caused this. I could fix that with solving the conflict, but that would be as if I made my own, new PR, so maybe that would be easier. What this PR does and still needs to be done in the 4.3-dev branch is to remove the obsolete assets for com_massmail and the "User Status" module, and to add the following assets: The other assets which this PR wants to add have already been added by my PR #33924 (modules 90, 96 to 98, 107, 108) |
@rdeutz Update: hmm wait i could be wrong and they are numbered like that so i got confused |
I've allowed myself to fix the conflicts in a way so it has as little impact as possible on other existing records but the PR still does what it shall do, remove the 2 obsolete assets and add the missing ones (which were complete with respect to @rdeutz 's list). I've moved the 'com_login' from id=12 to id=13. That could be safely done because there are no child assets of that asset, and that asset id was referenced nowhere, and so I could add the 'com_languages.language.1' asset at the right place regarding alphabetical order. Adding that asset at this place keeps the changes on lft and rgt values small. They change only up to the values of the removed, obsolete 'User Status' module's asset, after that they remain the same for other modules' assets. For the same reasons I have reused the asset id of the obsolete 'com_massmail' to add the missing 'com_mails' asset. The remaining missing assets have been added to the end of the tree, i.e. after so 'com_scheduler', so only the rgt value of the root asset needs to be updated to that. The PR should be tested by review by people who understand the nested table structure. |
Thx |
Remove unused com_massmail and added missing assets.
Testing instructions