AdminModule AdminInterfaceController - twig #2648

Kaik · 2015-11-22T18:40:57Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no*
Deprecations?	no
Tests pass?	no
Fixed tickets
Refs tickets	#1753
License	MIT
Doc PR	-
Changelog updated	yes

AdminInteraceController for new spec 2.0 modules using AbstractController and Twig.

Please take a look on it you can test it with this https://github.com/Kaik/core/tree/admin_settings_multilingual

Usage

{{ render(controller('ZikulaAdminModule:AdminInterface:header')) }}

This is still not finished do not merge.
Tasks

Move security etc private functions somewhere?..
Split notices into separate functions
Move breadcrumbs to separate function
Find place for moduleheader and modulelinks Extensions? module ModuleInterfaceController?..
AdminTab's breadcrumb's etc css and js improvements
Documentation
Improvements and fixes

Guite · 2015-11-22T19:09:04Z

src/system/AdminModule/Controller/AdminInterfaceController.php

+    			$i++;
+
+    			if (System::isLegacyMode() && !empty($class) && isset($menuitem['class'])) {
+    				if ($menuitem['class'] == 'z-icon-es-add') {


Maybe replace these if blocks by a mapping array checking like if (in_array($menuitem['class'], array_keys($oldIconMap))).

Yes this was copy & paste many thing's still not working I will be doing all issues you will point out so please comment what you think could be solved differently. This is very initial state we will need to work on it. I will update PR with issues and ideas to discuss.

yes please - replace this entire if/else block with something faster. even a switch block would be faster, but a mapping array is the best idea.

I have looked into this and I want to move all html to template - all css I will just pass them through controller to template with menu as array. I will also remove that old icon mapping as it is useless in case of twig modules we will just use icon class and drop mapping to "icon".

Guite · 2015-11-22T19:10:45Z

Very nice 👍

craigh · 2015-11-22T21:02:55Z

src/system/AdminModule/Controller/AdminInterfaceController.php

+    public function categorymenuAction($acid = null)
+    {
+
+    	if (!SecurityUtil::checkPermission('ZikulaAdminModule::', '::', ACCESS_ADMIN)) {


use new permissionApi - everywhere in this controller. I won't comment on every instance.

craigh · 2015-11-22T21:49:07Z

Find place for moduleheader and modulelinks Extensions? module ModuleInterfaceController?..

I don't understand what you mean here. Can you please explain more?

Kaik · 2015-11-22T21:51:39Z

moduleheader and modulelinks function are not only for admin for general purpose, those functions should be moved to other place best place is extension used to be in themes https://github.com/zikula/core/blob/1.4/src/system/ThemeModule/Resources/views/moduleheader.tpl

craigh · 2015-11-22T21:52:48Z

Aha. I see. These probably need to be converted to proper Twig tags but I will need to look more closely at them to be sure.

Kaik · 2015-11-23T12:39:45Z

Aha. I see. These probably need to be converted to proper Twig tags but I will need to look more closely at them to be sure.

Well there is more of them (smarty plugins for module use etc not this PR) and some for sure will be better do as Twig tags some using controller.
We can split them so they will be per module Controller/Twig...

AdminModule -> own AdminInterfaceController + Twig tags
ExtensionsModule -> own ExtensionInterfaceController + Twig tags

Eventually instead of {{ render(controller('ZikulaAdminModule:AdminInterface:header')) }}
We use more familiar {{ adminheader }}...

To consider... it is bugging my head so I will write it down...
I'm using request stack getMasterRequest to get calling module but might be better to move it to twig tag and pass to AdminInterfaceController another question is should it be possible to pass caller arguments into twig tag?

craigh · 2016-01-01T15:20:30Z

also can you convert to use template namespaces: https://github.com/zikula/core/blob/1.4/src/docs/Core-2.0/Performance.md#use-namespace-notation-for-templates

craigh · 2016-01-01T15:22:17Z

src/system/AdminModule/Controller/AdminInterfaceController.php

+            $update_show = true;
+            $update_version = $onlineVersion;
+        } else {
+            $update_show = false;


this (and line 247) overwrites all previous values of $update_show so why is it set previously?

Kaik · 2016-01-01T18:45:17Z

👍 Need to do few things before and then I will do this one and will implement it in blocks module few days 2-3

craigh · 2016-01-05T10:49:00Z

src/system/AdminModule/Resources/views/AdminInterface/categories.panel.html.twig

+{{ pageAddAsset('stylesheet', zasset('@ZikulaAdminModule:js/jQuery.mmenu-5.5.1/dist/core/css/jquery.mmenu.all.css')) }}
+{{ pageAddAsset('stylesheet', zasset('@ZikulaAdminModule:css/mmenu-hiddenpanel-customwidth.css')) }}
+
+{% macro draw(links, caller, module) %} {% for link in links %} 


Kaik · 2016-01-22T21:55:53Z

Hi, I have just spend day with this and have focused on updatecheck - this how it looks now.

first view when releases are listed and - its kind of active check
second view - passive check, when active check was performed previously (latest release was saved) and next check time has not yet been reached.

Kaik · 2016-01-22T23:56:48Z

Compleate collapsed state UpdateChecker status

Opened passive state

Opened active state

Opened active state with extended latest release information's

craigh · 2016-02-11T00:39:16Z

ping @Kaik

craigh · 2016-02-20T13:02:10Z

ping @Kaik

| Q | A | ----------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | no | Fixed tickets | - | Refs tickets | - | License | MIT | Doc PR | included | Changelog updated | yes - Add Twig tags and InterfaceController for Core-2.0 modules-AdminModule. - Add Twig tags and InterfaceController for Core-2.0 modules-ExtensionsModule. - Refactor to Twig and improve templates for headers, breadcrumbs, footers, menue's and links -AdminModule and ExtensionsModule.

craigh · 2016-02-27T16:47:17Z

to clear up the StyleCI issues, click on the Details link and then click the Login link at the top (can't remember here, but you may need to go through an OAuth procedure allowing StyleCI access to your Github stuff). Then once logged in, click "create Fix PR" and follow through with that. Then you have to merge the PR to your branch and delete the PR Fix branch. Then PULL your repository from github again to make sure you have the newest changes. Works great - especially in situations like this where there are many changes. If there are only a few, I will sometimes just manually fix the issues and push the fix myself. You can see I have done both methods on my current PR

Kaik · 2016-02-27T21:57:53Z

Ok this is cool I will try to fix some cs stuff manually, while I'm finishing things - basing on what is in details so I will learn/remember right cs. Anyway 2 things are left - update checker and that help function I will finish it tomorrow.

craigh · 2016-03-01T19:59:56Z

would really like to get this complete and integrated soon ( = this week), or it may have to wait for 1.4.3 which is another 4 months later. 👎

Kaik · 2016-03-02T17:06:54Z

I will finish this week.

craigh · 2016-03-07T22:33:57Z

Closing in favor of #2793

for the record - @Kaik did some great work. His major commit (which is still present as a standalone commit) and subsequent work is present in #2793. It only needed some fine tuning and fixing a few minor bugs.

Guite reviewed Nov 22, 2015
View reviewed changes

craigh reviewed Nov 22, 2015
View reviewed changes

craigh added this to the 1.4.2 milestone Nov 22, 2015

craigh added the Feature label Nov 22, 2015

craigh reviewed Jan 1, 2016
View reviewed changes

craigh reviewed Jan 5, 2016
View reviewed changes

Guite mentioned this pull request Jan 18, 2016

[Security] Too open file permissions warning for wrong file #2548

Closed

Kaik force-pushed the admin_module_twig branch from 9419b72 to 13c7ee8 Compare January 22, 2016 23:51

craigh mentioned this pull request Feb 3, 2016

Add support for 'account' type links in the LinkContainerCollector #2759

Merged

craigh force-pushed the 1.4 branch from 2ccf139 to eedd4aa Compare February 12, 2016 19:08

Kaik added 7 commits February 27, 2016 00:12

Use new AdminInterface Twig tag's in BlocksModule

715f190

Updatecheck

b911e8d

Updatecheck2

3092496

Updatecheck3

9107764

Updatecheck4

2e8240a

clean after rebase

bfe892d

Kaik force-pushed the admin_module_twig branch from 13c7ee8 to bfe892d Compare February 26, 2016 23:20

Move update check to separate class.

8b6a411

craigh closed this Mar 7, 2016

craigh mentioned this pull request Mar 7, 2016

Add Admin Interface Twig functions and controllers #2793

Merged

Kaik deleted the admin_module_twig branch March 15, 2016 18:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AdminModule AdminInterfaceController - twig #2648

AdminModule AdminInterfaceController - twig #2648

Kaik commented Nov 22, 2015

Guite Nov 22, 2015

Kaik Nov 22, 2015

craigh Nov 22, 2015

Kaik Dec 9, 2015

Guite commented Nov 22, 2015

craigh Nov 22, 2015

craigh commented Nov 22, 2015

Kaik commented Nov 22, 2015

craigh commented Nov 22, 2015

Kaik commented Nov 23, 2015

craigh commented Jan 1, 2016

craigh Jan 1, 2016

Kaik commented Jan 1, 2016

craigh Jan 5, 2016

Kaik commented Jan 22, 2016

Kaik commented Jan 22, 2016

craigh commented Feb 11, 2016

craigh commented Feb 20, 2016

craigh commented Feb 27, 2016

Kaik commented Feb 27, 2016

craigh commented Mar 1, 2016

Kaik commented Mar 2, 2016

craigh commented Mar 7, 2016

AdminModule AdminInterfaceController - twig #2648

AdminModule AdminInterfaceController - twig #2648

Conversation

Kaik commented Nov 22, 2015

Guite Nov 22, 2015

Choose a reason for hiding this comment

Kaik Nov 22, 2015

Choose a reason for hiding this comment

craigh Nov 22, 2015

Choose a reason for hiding this comment

Kaik Dec 9, 2015

Choose a reason for hiding this comment

Guite commented Nov 22, 2015

craigh Nov 22, 2015

Choose a reason for hiding this comment

craigh commented Nov 22, 2015

Kaik commented Nov 22, 2015

craigh commented Nov 22, 2015

Kaik commented Nov 23, 2015

craigh commented Jan 1, 2016

craigh Jan 1, 2016

Choose a reason for hiding this comment

Kaik commented Jan 1, 2016

craigh Jan 5, 2016

Choose a reason for hiding this comment

Kaik commented Jan 22, 2016

Kaik commented Jan 22, 2016

craigh commented Feb 11, 2016

craigh commented Feb 20, 2016

craigh commented Feb 27, 2016

Kaik commented Feb 27, 2016

craigh commented Mar 1, 2016

Kaik commented Mar 2, 2016

craigh commented Mar 7, 2016