Skip to content
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

Fix for nested sets in the menu (installation and update processes) #8284

Closed
wants to merge 3 commits into from
Closed

Fix for nested sets in the menu (installation and update processes) #8284

wants to merge 3 commits into from

Conversation

Kubik-Rubik
Copy link
Member

Please see #8274 and #8057 for more details.

We need to correct the nested sets after we've removed one entry in the menu.

Thanks to @matrikular and @roland-d for providing valued input on this topic.

How to test?

Do an installation and an update (with the changed files included in the package) and check whether the menu still works properly!

@Kubik-Rubik Kubik-Rubik added this to the Joomla! 3.5.0 milestone Nov 5, 2015
@waader
Copy link
Contributor

waader commented Nov 5, 2015

@Kubik-Rubik With #8274 I checked the component menus and the worked. So obviously that was not enough. I have to check all menu items, right?

@Kubik-Rubik
Copy link
Member Author

@waader It should also work with #8274 but the values are just not correct there. With this PR they were corrected.

@waader
Copy link
Contributor

waader commented Nov 5, 2015

Something I observed while getting prepared to test this patch. I installed the current staging branch and then went to Manage > Database. There I get this items:

able 'v1j2o_banners' does not have column 'alias' with type 'varchar(191)'. (From file 3.5.0-2015-07-01.sql.)
Table 'v1j2o_categories' does not have column 'alias' with type 'varchar(191)'. (From file 3.5.0-2015-07-01.sql.)
Table 'v1j2o_contact_details' does not have column 'alias' with type 'varchar(191)'. (From file 3.5.0-2015-07-01.sql.)
Table 'v1j2o_content' does not have column 'alias' with type 'varchar(191)'. (From file 3.5.0-2015-07-01.sql.)
Table 'v1j2o_menu' does not have column 'alias' with type 'varchar(191)'. (From file 3.5.0-2015-07-01.sql.)
Table 'v1j2o_newsfeeds' does not have column 'alias' with type 'varchar(191)'. (From file 3.5.0-2015-07-01.sql.)
Table 'v1j2o_tags' does not have column 'alias' with type 'varchar(191)'. (From file 3.5.0-2015-07-01.sql.)
Table 'v1j2o_ucm_content' does not have column 'core_alias' with type 'varchar(191)'. (From file 3.5.0-2015-07-01.sql.)

Can you adapt joomla.sql appropriately?

@Kubik-Rubik
Copy link
Member Author

@waader Thank you for the hint, I will fix it now.

@Kubik-Rubik
Copy link
Member Author

@waader @infograf768 Please test the update. Do a fresh installation and check whether database errors are displayed in "Extensions" - "Manage" - "Database".

@infograf768
Copy link
Member

Database Up to date. OK for me

@Kubik-Rubik
Copy link
Member Author

@infograf768 Thanks! And the menus are working properly? If yes, please mark as tested successfully. Thank you!

@waader
Copy link
Contributor

waader commented Nov 5, 2015

I have tested this item ✅ successfully on 03e6fd1

Thanks Kubik-Rubik!

I have tested mysql, postgres and mssql.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8284.

@wilsonge
Copy link
Contributor

wilsonge commented Nov 5, 2015

Can we try and split things up please. Let's keep all the stuff for the alias's in @zero-24 's PR #8268 (just PR the non-mysql database changes for that into there) and keep this specific to that menu change.

For the menu changes. The old values 18-20 were for weblinks. So any testers MUST make sure at minimum they test starting at 3.3.6 and upgrading through the versions to this version. To be honest whilst I don't mind what we start new installs on I see upgrading existing installs as very risky and bug prone - it could leave people with two items with the same lft value in a worst case scenario. This is why we have the rebuild button in the menu manager and don't do update queries for menus pretty much ever.

@Kubik-Rubik
Copy link
Member Author

@wilsonge Ah, I didn't see @zero-24's PR. I think then the best solution would be to close this PR and maybe add a notification to users that they should press the rebuild button once?

@waader
Copy link
Contributor

waader commented Nov 5, 2015

I didn´t see this neither. @Kubik-Rubik. you also have the date changes for postgres.

@Kubik-Rubik
Copy link
Member Author

I will close this PR in favor of #8268 and #8284 (comment).

@Kubik-Rubik Kubik-Rubik closed this Nov 5, 2015
@Kubik-Rubik Kubik-Rubik removed this from the Joomla! 3.5.0 milestone Nov 5, 2015
@Kubik-Rubik Kubik-Rubik deleted the nested_sets_menu branch November 5, 2015 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants