? Pending

User tests: Successful: Unsuccessful:

avatar laoneo
laoneo
27 Nov 2017

Pull Request for Issue #18685.

Summary of Changes

Loads the model and tables by creating the instances directly in the sample data blog plugin.

There is still an error in the nestedTable set which I didn't figure out on a first glance. Perhaps @Bakual can help out here.

avatar laoneo laoneo - open - 27 Nov 2017
avatar laoneo laoneo - change - 27 Nov 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 27 Nov 2017
Category Front End Plugins
avatar Bakual
Bakual - comment - 27 Nov 2017

I got Notice: Undefined index: tags in D:\xampp\htdocs\joomla4\libraries\src\MVC\Model\AdminModel.php on line 1133 which broke the JSON response.
I've then disabled the error reporting and then it failed on the second step due to You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ')\r\nGROUP BY c2.id) AS c2 ON c2.newId = c.id\r\nSETpublished= c2.newPublish' at line 6"

I guess there are some issues unrelated to the model fetching.

I tend to be also with Michael (#17705 (comment)) in that we should try to fix the getInstance method first instead of directly calling the class.

avatar wilsonge
wilsonge - comment - 27 Nov 2017

OK I fixed the malformed JSON with 5e462ec - @laoneo if you rebase you should be able to fix that part. Don't have time to look into the mysql failure right now - i'll leave that for one of you two :)

avatar laoneo laoneo - change - 28 Nov 2017
Labels Added: ?
avatar laoneo
laoneo - comment - 28 Nov 2017

Merged the changes.

@Bakual I fear that this will be a long discussion in #17705 where it will take some time till it get implemented and merged. I did a quick search and the sample data plugin is the only place where JModelLegacy::getInstance is called. So it would probably not bad to fix the sample data plugin first that people can play around with some sample data in J4.

avatar Bakual
Bakual - comment - 28 Nov 2017

I did a quick search and the sample data plugin is the only place where JModelLegacy::getInstance is called.

While that is true, in J3 there are 42 instances of that code (also quick search). What likely happend is that while rewriting the stuff, one did the exact same thing you propose here and the models are now created directly instead of going through the (broken) getInstance method.
So it was not "the only place". It's just the only place left that isn't changed to the "workaround". ?

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 29 Nov 2017

@Shedyk can you please test this?

avatar infograf768
infograf768 - comment - 29 Nov 2017

Articles and categories are created, but not the menu items. Home remains as featured after clean install and enabling only the languagefilter plugin
Other issue: the install bar does not stop moving.

avatar laoneo
laoneo - comment - 30 Nov 2017

There is still a db error, which I hope @Bakual can help out here. Not sure if it is related to J4 or my changes.

avatar Bakual
Bakual - comment - 30 Nov 2017

@laoneo Found one thing, but then the next error popped up:
Warning: Joomla\CMS\Table\Menu::store(): Couldn't fetch mysqli_result in D:\xampp\htdocs\joomla4\libraries\src\Table\Menu.php on line 281 ?

avatar laoneo
laoneo - comment - 30 Nov 2017

With my latest commits, I could finally install sample data. @Bakual can you check if the sample data is correct displayed on the front end?
For me the progress bar is not hiding and the returned messages are not displayed. But that can be solved in another pr, important is to have sample data available that people can actually play around with J4.

avatar laoneo laoneo - change - 30 Nov 2017
Title
[4.0] Fix to load the models and tables correct in the sample data blog plugin
[4.0] Fix the sample data blog plugin
avatar laoneo laoneo - edited - 30 Nov 2017
avatar Bakual
Bakual - comment - 30 Nov 2017

Yep, seems to work now. The error I had previously probably was a glitch, it doesn't happen anymore.

What I get thought is a warning in frontend "The template for this display is not available. Please contact a Site administrator."

The progress bar and the corresponding JS code needs to be looked at. I guess that is because of the move to BS4 or removal of jQuery use or whatever. Maybe @Fedik can have a look, he helped me originally with the code I think.

avatar infograf768
infograf768 - comment - 30 Nov 2017

I still get an error here when clicking on the Author Login menu item
screen shot 2017-11-30 at 10 39 08

Concerning the bar, I suggest we also get a success message when the data has finished installing.

Note for future work: As it looks like multilingual sample data is no more possible to install when installing Joomla, I guess we will have to consider now a new real multilingual sample data that would consider the languages installed during installation or later on through the administration.
To make it easy on users, that sample should do something very similar to what we have in 3.x, including enabling languagefilter, creating a mod_languages module, etc.

avatar infograf768
infograf768 - comment - 30 Nov 2017

concerning the error above, When creating another j4 site and such a menu item I have no issue.
Weird...

avatar Bakual
Bakual - comment - 30 Nov 2017

Concerning the bar, I suggest we also get a success message when the data has finished installing.

Those are related. It's the same JS which does update the bar and show the messages.

avatar infograf768
infograf768 - comment - 30 Nov 2017

Concerning the error with Author login, I do not get it when not installing this sample data.

avatar Bakual
Bakual - comment - 30 Nov 2017

There are two "Author Login" menuitems which are slightly different: One has a login redirect set. Which one does give you the issue?
And what happens when you re-save the menuitem?

avatar infograf768
infograf768 - comment - 30 Nov 2017

Both give the same result.
deleting them, rebuilding menu items and then creating a new login menu item does not solve the issue.

avatar infograf768
infograf768 - comment - 30 Nov 2017

FYI, when I change the menu type of a menu item (from blog to main for example), I get errors

[30-Nov-2017 11:42:56 Europe/Berlin] PHP Notice:  Undefined index: parent_id in /Applications/MAMP/htdocs/installmulti/joomla40/administrator/components/com_menus/Model/ItemModel.php on line 1366
[30-Nov-2017 11:42:56 Europe/Berlin] PHP Notice:  Undefined index: parent_id in /Applications/MAMP/htdocs/installmulti/joomla40/administrator/components/com_menus/Model/ItemModel.php on line 1393
avatar infograf768
infograf768 - comment - 30 Nov 2017

FYI, when saving a menu item I get

[30-Nov-2017 11:41:38 Europe/Berlin] PHP Notice:  Undefined index: parent_id in /Applications/MAMP/htdocs/installmulti/joomla40/administrator/components/com_menus/Model/ItemModel.php on line 1366
[30-Nov-2017 11:41:38 Europe/Berlin] PHP Notice:  Undefined index: parent_id in /Applications/MAMP/htdocs/installmulti/joomla40/administrator/components/com_menus/Model/ItemModel.php on line 1393

also when using the link Site Settings, I get
0 Invalid controller class: Joomla\Component\Config\Site\Controller\Config.display.configController

for Template Setttings
0 Invalid controller class: Joomla\Component\Config\Site\Controller\Config.display.templatesController

for Change Password or Log Out
0 Class 'ContentHelperRoute' not found

avatar laoneo
laoneo - comment - 30 Nov 2017

Arer these issues from the sample data plugin or from J4 itself?

avatar infograf768
infograf768 - comment - 30 Nov 2017

i did not test for other menu items than login, but, yes, for that one it looks like it is due to the sample data plugin.
will test for the others after siesta.

undefined parent_id looks like a pure j4 thing.

avatar Bakual
Bakual - comment - 30 Nov 2017

We may have to define that when creating the menuitem. In J3 it probably worked because we don't use strict mode there.

avatar infograf768
infograf768 - comment - 1 Dec 2017

Tested the menu items Site Configuration Options and Template options with public access (yes, I know they should only be used by a registered user with back-end acces) on a clean J4 install.

We do have a different error if the user is not logged in (tested with SuperUser).
0 Class 'Notallowed' not found
So that is a different issue and displaying a readable error would be nice. Something like "You have not access... etc.)

Once their access is set correctly, they do work fine.

After installing sample data blog I have the errors stated above, and also for the same menu items I created before installing it

Therefore I am now convinced that the plugin corrupts in a way or another the menu table.

I checked the table in 3.8 and the one created on J4 and I did not remark differences for these menu items.

avatar Bakual
Bakual - comment - 1 Dec 2017

It's actually caused by the modules "Most Read Posts" and "Older Posts". Once you disable those, it works.

avatar Bakual
Bakual - comment - 1 Dec 2017

See #18953 for the module fix

avatar infograf768
infograf768 - comment - 2 Dec 2017

#18953 works for all (Great!) except Site Settings and Template Settings

avatar infograf768
infograf768 - comment - 2 Dec 2017

Ok, I found the issue

The links have changed in 4.0. We have an update sql which changes the link.

UPDATE `#__menu` SET `link` = 'index.php?option=com_config&view=config' WHERE `link` = 'index.php?option=com_config&view=config&controller=config.display.config';
UPDATE `#__menu` SET `link` = 'index.php?option=com_config&view=templates' WHERE `link` = 'index.php?option=com_config&view=templates&controller=config.display.templates';

@laoneo
Can you modify the lines 551 and 564 with the new links? we should have

		// Insert menuitems level 2.
		$menuItems = array(
			array(
				'menutype'     => $menuTypes[1],
				'title'        => JText::_('PLG_SAMPLEDATA_BLOG_SAMPLEDATA_MENUS_ITEM_9_TITLE'),
				'link'         => 'index.php?option=com_config&view=config',
				'parent_id'    => $menuIdsLevel1[4],
				'component_id' => 23,
				'access'       => 6,
				'params'       => array(
					'menu_text'         => 1,
					'show_page_heading' => 0,
					'secure'            => 0,
				),
			),
			array(
				'menutype'     => $menuTypes[1],
				'title'        => JText::_('PLG_SAMPLEDATA_BLOG_SAMPLEDATA_MENUS_ITEM_10_TITLE'),
				'link'         => 'index.php?option=com_config&view=templates',
				'parent_id'    => $menuIdsLevel1[4],
				'component_id' => 23,
				'params'       => array(
					'menu_text'         => 1,
					'show_page_heading' => 0,
					'secure'            => 0,
				),
			),
		);
avatar wilsonge
wilsonge - comment - 2 Dec 2017

That makes sense - we swapped com_config back to old MVC

avatar laoneo
laoneo - comment - 2 Dec 2017

Done.

avatar wilsonge wilsonge - close - 2 Dec 2017
avatar wilsonge wilsonge - merge - 2 Dec 2017
avatar wilsonge wilsonge - change - 2 Dec 2017
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-12-02 15:54:58
Closed_By wilsonge
avatar wilsonge
wilsonge - comment - 2 Dec 2017

OK Let's run with this for now. I'm still very unsure about this direct creation. But it works for now

avatar Bakual
Bakual - comment - 2 Dec 2017

@wilsonge To be fair, it was done like this through the whole CMS.

avatar wilsonge
wilsonge - comment - 2 Dec 2017

I know. My aim was to use the MVCFactory throughout. But somehow it's only been done that way inside a given component. I dunno. needs thought

avatar laoneo
laoneo - comment - 3 Dec 2017

As pointed out here #17705 (comment), I see two approaches to work with MVCFactories of other components MVC classes.

Add a Comment

Login with GitHub to post a comment