You will also need some code to fix existing assets

You will also need some code to fix existing assets

Users testing the release and particularly tours could just re-save their tours to have the parent asset ids fixed.

You will also need some code to fix existing assets

Users testing the release and particularly tours could just re-save their tours to have the parent asset ids fixed.

that is not acceptable to me, especially as it is undocumented, as many years ago we switched the policy so that you can upgrade etc from beta and rc releases.

You will also need some code to fix existing assets

Users testing the release and particularly tours could just re-save their tours to have the parent asset ids fixed.

that is not acceptable to me, especially as it is undocumented, as many years ago we switched the policy so that you can upgrade etc from beta and rc releases.

@brianteeman I don't see a way to fix that e.g. with an update SQL script.

@brianteeman I don't see a way to fix that e.g. with an update SQL script.

one option might be to simply delete all existing guided tours assets. better to have none than to have broken ones

@brianteeman I don't see a way to fix that e.g. with an update SQL script.

one option might be to simply delete all existing guided tours assets. better to have none than to have broken ones

We could also add this mention to the FAQ for the release, that is what the FAQ can be used for.

I've found a mistake in the assets table: The rgt value of the root item is 175 but should be 177. This means that when you test this PR here, the new asset is inserted correctly, but the rgt value of the root item is not updated accordingly. I will make a separate PR to fix that.

Update: I've found another mistake with the lft and rgt values in the assets table.

I have tested this item ✅ successfully on 120174b

Asset is created with the right parent when editing a tour for the first time or creating a new one. When finally deleting a trashed tour, the right asset is deleted, too.

@brianteeman I think if we do something for deleting wrong assets like you suggested, we should do it with a separate PR.

@brianteeman I think if we do something for deleting wrong assets like you suggested, we should do it with a separate PR.

fine by me - as long as it gets done and we dont screw up users sites by not doing it

@brianteeman I think if we do something for deleting wrong assets like you suggested, we should do it with a separate PR.

fine by me - as long as it gets done and we dont screw up users sites by not doing it

There is one thing I don't grasp here: why would we screw user sites? We are not in stable release yet.

I don't see either how it would screw sites. It would only mean there are some unused, obsolete assets when someone has updated from a 4.3.0 Beta 4 or RC 1.

What we could do is to delete these assets since gaps in the lft and rgt values are not doing harm, like this:

DELETE FROM `#__assets` WHERE `parent_id` = 1 AND `level` = 1 AND `name` LIKE 'com_guidedtours.tour.%';

It should be safe since we touch only the wrong ones.

But is it worth that? It's always a risk to touch user data on update.

As we deal with users testing sites, I would think a FAQ info would suffice, the FAQ is supposed to report issues like these.
I would prefer telling testers this issue is fixed if you just save tours rather than removing the permissions they have set up and having an asset table a bit messed up.

In the past we had a policy of not providing an upgrade path from beta and rc. This policy was changed. It is not ideal that the only option appears to be to delete the existing assets but if thats the only option it will have to do. You can not rely on someone reading an faq.

Those with a long memory will remember that this is not the first time we have had an issue arising from a bad entry in an asset table. We had to create a script for users to run top resolve it BUT that was only after a long time before the weird bug was identified and traced back to the bad assets

I understand, it makes sense, I get that.
But we are dealing with tours here, nothing that has a major impact on anything else in the core or any frontend.

Looks to me that if we delete the faulty lines in the asset table, we mess up the asset table. If we don't do anything, the parent ids are messed up unless they get the info that they just need to save the tours again.
There is no ideal solution either way.

But we are dealing with tours here, nothing that has a major impact on anything else in the core or any frontend.

If this was in a tours table then I would agree but its not.

Please read https://docs.joomla.org/Fixing_the_assets_table

As of Joomla 1.6 there is an #__assets table that might have improper values. This can cause serious problems for your site.

Thinking about it - we have a rebuild assets functionality for categories and menus already. cant we do the same for tours (except do it on upgrades and not a manual button

Thinking about it - we have a rebuild assets functionality for categories and menus already. cant we do the same for tours (except do it on upgrades and not a manual button

@Do these buttons really rebuild the assets table? I think they only rebuild the (also nested) menu and categories tables.

May I pose the question why we need assets at all? The tours to me are rather like banners and since we don't have categories where it would be usefull to setup inherited permissions, I don't see a reason for polluting our assets table and introducing all this additional computation.

Thinking about it - we have a rebuild assets functionality for categories and menus already. cant we do the same for tours (except do it on upgrades and not a manual button

@Do these buttons really rebuild the assets table? I think they only rebuild the (also nested) menu and categories tables.

Those buttons only rebuild the nested set structure of the respective database table. We do need that for the assets table, too, but not in the context of this component.

May I pose the question why we need assets at all? The tours to me are rather like banners and since we don't have categories where it would be usefull to setup inherited permissions, I don't see a reason for polluting our assets table and introducing all this additional computation.

It would allow the administrator of a site to lock some tours and allow the creation and edition of others, as an example.

I have tested this item ✅ successfully on 120174b

I don't know what is the best way to handle existing wrong assets, but this fix is needed for tours permission works properly. That's why I give it a success test.

RTC

May I pose the question why we need assets at all? The tours to me are rather like banners and since we don't have categories where it would be usefull to setup inherited permissions, I don't see a reason for polluting our assets table and introducing all this additional computation.

It would allow the administrator of a site to lock some tours and allow the creation and edition of others, as an example.

Not exactly. The way it is set now, you could only lock down the component and then allow single tours to be edited or deleted. But you can't create tours without the components global create permission. I don't see a need to lock down all tours except for specific ones for editing or deleting. Considering all that, I would rather remove this again before the 4.3.0 release. We can add this back in if the need turns out to be there in a later release, but we will never be able to remove it again once it has been released.

See an alternative PR here, which removes the permissions for tours: #40228 .

The PR for the errors which I've found in the assets table (see my comment above) is #40237 . It will be ready when I have completed testing instructions, hopefully later tonight.

P.S.: It is independent from having assets for the tours or not.

Closed in favor of #40228.