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

[4.0] Defer loading of menu items until required #28635

Merged
merged 1 commit into from Apr 11, 2020

Conversation

wilsonge
Copy link
Contributor

Pull Request for Issue #28614 .

Summary of Changes

Defers loading of the menu items until we actually need the information on the menu items. In certain cases this will mean that

Testing Instructions

  1. Ensure menus load in the frontend on a normal single language site
  2. Ensure menus load in the frontend on a multilanguage site
  3. Ensure menus load in the backend
  4. Update Joomla from 3.10 (multilanguage site) using the PR custom update site to this branch. It should now work after this patch.

Documentation Changes Required

Yes. Document menu items are being loaded later. This will only be relevant however for custom menus extending Joomla\CMS\Menu\AbstractMenu so should be an edge case.

@wilsonge wilsonge changed the title Defer loading of menu items until required [4.0] Defer loading of menu items until required Apr 11, 2020
@alikon
Copy link
Contributor

alikon commented Apr 11, 2020

I have tested this item ✅ successfully on f971e00

with mysql 8.0.19 and postgresql


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

@alikon
Copy link
Contributor

alikon commented Apr 11, 2020

btw it seems that we have still an issue with PDO cause have worked even before this pr
so we still have soemething to fix on the PDO drivers

@Razzo1987
Copy link
Contributor

I have tested this item ✅ successfully on f971e00

Tested on:

dbtype mysqli

Database Type mysql
Database Version 10.1.43-MariaDB
Database Collation utf8_general_ci
Database Connection Collation utf8mb4_general_ci
Database Connection Encryption None
Database Server Supports Connection Encryption No
PHP Version 7.3.12
Web Server Apache
WebServer to PHP Interface fpm-fcgi


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

@alikon
Copy link
Contributor

alikon commented Apr 11, 2020

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 11, 2020
@infograf768
Copy link
Member

I have tested this item ✅ successfully on f971e00

Solves isssue for sqli as well as sql(PDO)


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

Copy link
Member

@infograf768 infograf768 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

@richard67
Copy link
Member

@wilsonge Please wait a bit with merging, am just starting with tests ... even if it has already 2 good tests. But we should be very careful here.

@richard67
Copy link
Member

That's one of the PR's (update tests) where I am really happy about the update packages built by drone for this PR. In pasz I had to build them myself "manually".

@richard67
Copy link
Member

I have tested this item ✅ successfully on f971e00


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

@wilsonge wilsonge merged commit fde5545 into joomla:4.0-dev Apr 11, 2020
@wilsonge
Copy link
Contributor Author

Thankyou guys!

@wilsonge wilsonge deleted the feature/defer-menu-load branch April 11, 2020 09:19
@joomla-cms-bot joomla-cms-bot added PR-4.0-dev and removed RTC This Pull Request is Ready To Commit labels Apr 11, 2020
@wilsonge wilsonge added this to the Joomla 4.0 milestone Apr 11, 2020
@richard67
Copy link
Member

Thank you guy 😛

@infograf768
Copy link
Member

drone and appveyor both failing.
hard to figure what's going on...

@richard67
Copy link
Member

Appveyor is a known issue ... Hannes is on it.
Drone seems to be the regular randomly happening timeout. We will see.

@SharkyKZ
Copy link
Contributor

Please test #28647 for timeout issue.

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

7 participants