User tests: Successful: Unsuccessful:
Presently there is no tracking in the database of a relationship between a package extension and the extensions it installs, really the only way to know this information is to process the package extension's XML manifest. This is a step at improving this so we can do other things throughout the API.
With the patch applied, installing a package extension (language packages are great for testing here) should record the package extension's ID to the #__extensions
table's new package_id
column. A query failure setting this info is gracefully handled and essentially results in the current status.
This query will be run during the finaliseInstall
step of the install process. This step is run by the install
and update
paths (discover_install
as well but packages don't support this so it's a mute point), so as extension packages are updated after this change is deployed, packages will start to correctly associate themselves with child extensions.
Note the newly added database column.
Status | New | ⇒ | Pending |
Category | ⇒ | Administration Language & Strings SQL Installation Libraries |
Another related feature for this would be to be able to set dependencies between packages. That would allow to have a better system for #12961 as well, where extensions could only be locked if they are being used by another extension.
Labels |
Removed:
?
?
|
Category | Administration Language & Strings SQL Installation Libraries | ⇒ | SQL Administration com_admin Postgresql MS SQL com_installer Language & Strings Templates (admin) Installation Libraries |
Title |
|
Title |
|
Added all the required schema. For what this PR is it is now feature complete.
what about to report this information (for example in Extensions: Manage) ?
Done.
Right now I'm basically locking my own extensions from the package so they cannot be individually removed (without removing the whole package). Its a hack and I don't like it, so I'd want to get rid of it if possible.
As I hinted at in #12976 a later phase would introduce API to be able to manage this. I haven't thought this through completely yet, but shooting from the hip, it would basically be if you choose to uninstall an extension that has an associated package, it would load that package's data to determine if the package allows individual uninstalls (probably a tag in the manifest schema or an attribute on the base tag similar to method="upgrade"
, to be determined and programmatically defaulting to existing behavior) and if the package disallows it then the process is aborted. This way individual extensions don't have to make these types of checks on their own (as their minimums come up to where Joomla has this API) and we can remove ugly hacks like what's described in that issue which blocks removing language extensions that are installed without a corresponding package.
Another related feature for this would be to be able to set dependencies between packages. That would allow to have a better system for #12961 as well, where extensions could only be locked if they are being used by another extension.
Inter-extension dependency declarations/tracking isn't in any way related to this. Nor should it be.
Nice work @mbabker :) I cannot wait removing my ugly hack on this.
Another question that comes into my mind is: what if package provided optional extension that can be removed and the user chooses to remove it. After a while there's a new version of the package -- so user installs it. Right now the removed extension would not stay uninstalled.
Though its a bit more complicated than this as new extensions could also be added to the package later on. And what would happen if user changes his mind and wants to get it back...
A package should always install all parts of the package imho. If the user uninstalls a part, then it should be reinstalled. That's what I would expect.
i've made some test with different pkg's like pkg_weblinks ect...
but the new field package_id
is never stored
some bug on installer/adapter ?
I'd call that a bug in JTable then if the PK field isn't getting populated on insert.
same results as @alikon installed several pkgs and i never get the package_id populated.
Also some other things i noticed:
0
so all sites have the default 0
value?pkg_en-GB
and en-GB languages have that relation on install and on update?1%
and also with nowrap classi never get the package_id populated
Then there is a bug with the use of JTable populating the PK fields after an insert operation. Unrelated to this pull request. I will not change to use $db->insertid()
as we have no way of guaranteeing that the last identifier is indeed the one for the installed extension. Other suggestions are welcome otherwise this bug alone causes this pull request to be unacceptable.
Shouldn't the update sql update all package id to 0 so all sites have the default 0 value?
That's exactly what the NOT NULL DEFAULT 0
segment of the SQL statement does. If we have to execute a manual UPDATE
statement after altering the table then the use of this construct cannot be relied on in general and the obvious bug should be reported to the upstream database engines.
Shouldn't the existing pkg_en-GB and en-GB languages have that relation on install and on update?
We cannot guarantee the 802 ID is the correct relation.
The package_id column should, IMHO, be width 1% and also with nowrap class
This is contradictory. The column's "native" width at 1% is 54px, adding the nowrap
class makes it 72px. In other words there is no way for the column with its title to have a 1% width and nowrap
as the class declaration extends it out to 1.33%. Without any style declarations, the column assumes a "natural" width of 80 pixels and with the arrow it extends out to 104 pixels.
The package_id ordering column doesn't show the arrow on order
Fixed.
Then there is a bug with the use of JTable populating the PK fields after an insert operation. Unrelated to this pull request. I will not change to use $db->insertid() as we have no way of guaranteeing that the last identifier is indeed the one for the installed extension. Other suggestions are welcome otherwise this bug alone causes this pull request to be unacceptable.
yes i understand is not related to this PR, but the fact is that needs to somehow be fixed, because it doesn't seem to work as it is
That's exactly what the NOT NULL DEFAULT 0 segment of the SQL statement does. If we have to execute a manual UPDATE statement after altering the table then the use of this construct cannot be relied on in general and the obvious bug should be reported to the upstream database engines.
right, sleeping sorry
We cannot guarantee the 802 ID is the correct relation.
you can, on install sql, but not on update, but you can still do a query on update to check for the pkg-en-GB id and use it.
This is contradictory. The column's "native" width at 1% is 54px, adding the nowrap class makes it 72px.
1% makes the column smallest as possible according to the viewport width.
nowrap makes to column never wrap.
https://github.com/joomla/joomla-cms/blob/staging/administrator/templates/isis/css/template.css#L5591
yes i understand is not related to this PR, but the fact is that needs to somehow be fixed, because it doesn't seem to work as it is
Turns out it's not a bug in JTable. The JTableExtension
instance used by the JInstaller
instance is not the same as the one in the JInstallerAdapter
object. So, to get the installed extension ID, this now has to hook into the onExtensionAfterInstall
event which broadcasts the JInstaller
instance and the result of JInstallerAdapter::install()
. Funny enough, that's documented to only return boolean, but it actually returns boolean false on a fail condition and the extension ID on success (yay documentation bugs).
As for the column width, honestly I'd have to say we're doing it majorly wrong if we're declaring in the HTML that it should be the smallest width then in the CSS declarations using a class set that is widening that column beyond what the HTML has declared. The HTML width declaration should be consistent with what we are doing with design otherwise why bother with the width attributes in the first place? So if I add this stuff, I will do it only using values that are consistent with the end result.
As for the column width, honestly I'd have to say we're doing it majorly wrong if we're declaring in the HTML that it should be the smallest width then in the CSS declarations using a class set that is widening that column beyond what the HTML has declared. The HTML width declaration should be consistent with what we are doing with design otherwise why bother with the width attributes in the first place? So if I add this stuff, I will do it only using values that are consistent with the end result.
Show all checks
that is not big issue. not worth pending this PR because of that IMHO
reggarding the update sql query for pkg_en-GB
something like this would work, i think
UPDATE `#__extensions` AS `e1`
INNER JOIN (SELECT `extension_id` FROM `#__extensions` WHERE `type` = 'package' AND `element` = 'pkg_en-GB') AS `e2`
SET `e1`.`package_id` = `e2`.`extension_id`
WHERE ((`e1`.`type`= 'language' AND `e1`.`element` = 'en-GB') OR (`e1`.`type`= 'package' AND `e1`.`element` = 'pkg_en-GB'));
Update queries done (I'll be so glad when we stop including data changes in the schema changeset files). If someone wants to make sure they're actually right for all engines that'd be helpful.
misses the pkg_en-GB relation in joomla.sql install files
I have tested this item
run updated sql manually installed language packages and weblinks and package id is there.
I have tested this item
on postgresql
Status | Pending | ⇒ | Ready to Commit |
Labels |
RTC
Milestone |
Added: |
Done
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-12-07 08:29:55 |
Closed_By | ⇒ | rdeutz |
A relationship between a package extension and the extensions it installs query-able via SQL is a good thing,
a little bit beyond
what about to report this information (for example in Extensions: Manage) ?