PR-4.3-dev Failure

User tests: Successful: Unsuccessful:

avatar fastslack
fastslack
24 Jul 2020

Remove unused com_massmail and added missing assets.

Testing instructions

  1. Install Joomla! as usual
  2. Check if everything works fine.
avatar fastslack fastslack - open - 24 Jul 2020
avatar fastslack fastslack - change - 24 Jul 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 24 Jul 2020
Category SQL Installation
avatar richard67
richard67 - comment - 24 Jul 2020
  1. Fix for PostgreSQL is missing.
  2. Testing instructions are missing.
  3. Reference to the issue is missing. I assume it's issue #30179 .

Please add the missing information.

avatar fastslack fastslack - change - 24 Jul 2020
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 24 Jul 2020
Category SQL Installation SQL Installation Postgresql
avatar fastslack fastslack - change - 24 Jul 2020
Title
Fixing #__assets table
[4.0] Fixing #__assets table
avatar fastslack fastslack - edited - 24 Jul 2020
avatar fastslack
fastslack - comment - 24 Jul 2020

Added the PostgreSQL changes and testing instruction

avatar fastslack fastslack - change - 24 Jul 2020
The description was changed
avatar fastslack fastslack - edited - 24 Jul 2020
avatar fastslack fastslack - change - 24 Jul 2020
The description was changed
avatar fastslack fastslack - edited - 24 Jul 2020
avatar fastslack fastslack - change - 24 Jul 2020
The description was changed
avatar fastslack fastslack - edited - 24 Jul 2020
avatar wilsonge
wilsonge - comment - 11 Aug 2020

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

avatar ghost
ghost - comment - 12 Sep 2020

Patchtester: After "Apply Patch":

Screenshot_2020-09-12 Joomla Patch Tester - fw40 - Administration

avatar brianteeman
brianteeman - comment - 12 Sep 2020

@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

avatar richard67
richard67 - comment - 12 Sep 2020

@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.

avatar brianteeman
brianteeman - comment - 12 Sep 2020

where the folder has to be removed at the end.

It does not have to be removed with a beta

avatar drmenzelit
drmenzelit - comment - 2 Dec 2021

@fastslack could you solve the conflicts?

avatar brianteeman
brianteeman - comment - 14 Jun 2022

Is this still relevant? Looks to me like it was addressed by @richard67 in #33924 and can be closed?

avatar richard67
richard67 - comment - 14 Jun 2022

Well the asset for the mass mail component was not removed with my PR.

avatar brianteeman
brianteeman - comment - 14 Jun 2022

sorry - i confused mailto and massmail

avatar alikon
alikon - comment - 17 Jun 2022

btw this has become obsolete in the meantime as for an example com_csp don't exist
https://github.com/joomla/joomla-cms/pull/30178/files#diff-3a9a51c74db65d66b70dbd56c86e63882cfaa26701ef6ff3f88bd0ea2682bb9fR114

avatar HLeithner
HLeithner - comment - 27 Jun 2022

This pull request has automatically rebased to 4.2-dev.

avatar rdeutz rdeutz - change - 21 Oct 2022
Title
[4.0] Fixing #__assets table
Fixing #__assets table
avatar rdeutz rdeutz - edited - 21 Oct 2022
avatar rdeutz
rdeutz - comment - 21 Oct 2022

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
com_admin
com_ajax
com_associations
com_banners
com_banners.category.3
com_cache
com_categories
com_checkin
com_config
com_contact
com_contact.category.4
com_content
com_content.category.2
com_content.state.1
com_content.transition.1
com_content.transition.2
com_content.transition.3
com_content.transition.4
com_content.transition.5
com_content.transition.6
com_content.transition.7
com_content.workflow.1
com_contenthistory
com_cpanel
com_fields
com_finder
com_installer
com_joomlaupdate
com_languages
com_languages.language.1
com_login
com_mails
com_media
com_menus
com_menus.menu.1
com_messages
com_modules
com_modules.module.1
com_modules.module.10
com_modules.module.100
com_modules.module.101
com_modules.module.102
com_modules.module.103
com_modules.module.104
com_modules.module.105
com_modules.module.106
com_modules.module.107
com_modules.module.108
com_modules.module.12
com_modules.module.15
com_modules.module.16
com_modules.module.17
com_modules.module.2
com_modules.module.3
com_modules.module.4
com_modules.module.79
com_modules.module.8
com_modules.module.86
com_modules.module.87
com_modules.module.88
com_modules.module.89
com_modules.module.9
com_modules.module.90
com_modules.module.91
com_modules.module.92
com_modules.module.93
com_modules.module.94
com_modules.module.95
com_modules.module.96
com_modules.module.97
com_modules.module.98
com_modules.module.99
com_newsfeeds
com_newsfeeds.category.5
com_plugins
com_postinstall
com_privacy
com_redirect
com_scheduler
com_tags
com_templates
com_users
com_users.category.7
com_workflow
com_wrapper
root.1

avatar richard67
richard67 - comment - 21 Oct 2022

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:
com_associations
com_categories
com_fields
com_languages.language.1 (that's the "English (en-GB)" content language)
com_mails'
com_workflow'

The other assets which this PR wants to add have already been added by my PR #33924 (modules 90, 96 to 98, 107, 108)

avatar richard67
richard67 - comment - 21 Oct 2022

@rdeutz How did you create your list? are you sure we have a "com_newsfeeds.category.5" in a plain new 4.3-dev core installation? Or "com_banners.category.3"? Or "com_contact.category.4"? To me that looks pretty much like the result from a database where sample data was installed, where data is created dynamically with PHP so we definitely cannot and should not provide static asset ids in our base.sql.

Update: hmm wait i could be wrong and they are numbered like that so i got confused

avatar richard67 richard67 - change - 21 Oct 2022
Labels Added: Conflicting Files ? ? PR-4.3-dev
Removed: ?
avatar richard67
richard67 - comment - 21 Oct 2022

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.

avatar bembelimen bembelimen - change - 23 Oct 2022
Labels Removed: Conflicting Files ? ?
avatar bembelimen bembelimen - change - 23 Oct 2022
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2022-10-23 09:28:01
Closed_By bembelimen
avatar bembelimen bembelimen - close - 23 Oct 2022
avatar bembelimen bembelimen - merge - 23 Oct 2022
avatar bembelimen
bembelimen - comment - 23 Oct 2022

Thx

Add a Comment

Login with GitHub to post a comment