Skip to content

[4.0] Module position no more empty when coming from CPanel#24583

Closed
infograf768 wants to merge 6 commits intojoomla:4.0-devfrom
infograf768:4.0_modules_cpanel_userstate
Closed

[4.0] Module position no more empty when coming from CPanel#24583
infograf768 wants to merge 6 commits intojoomla:4.0-devfrom
infograf768:4.0_modules_cpanel_userstate

Conversation

@infograf768
Copy link
Member

Pull Request for Issue #24575

Summary of Changes

Forcing userstate to administrator modules

Testing Instructions

Load Site Modules Manager to be sure userstate is set to site modules
Switch to Control Panel
Edit a module through the cog

Before patch

The module position is empty

After patch

It is now correctly using cpanel position

@chmst @franz-wohlkoenig

@chmst
Copy link
Contributor

chmst commented Apr 14, 2019

I have tested this item ✅ successfully on a0846e4


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

@infograf768
Copy link
Member Author

Solved conflicts.
One more tester.

<<?php echo $moduleTag; ?> class="card mb-3<?php echo $moduleClassSfx; ?>">
<?php if ($canEdit || $canChange) : ?>
<?php // Make sure we get the admin template module positions ?>
<?php Factory::getApplication()->setUserState('com_modules.modules.client_id', 1, 'int'); ?>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary 'int'?

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure

Copy link
Contributor

Choose a reason for hiding this comment

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

Checked similar calls and they don't include 'int'.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@infograf768 infograf768 force-pushed the 4.0_modules_cpanel_userstate branch from a0f61bd to 286b10c Compare April 15, 2019 04:48
@Quy
Copy link
Contributor

Quy commented Apr 15, 2019

I have tested this item ✅ successfully on 286b10c


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

@ghost
Copy link

ghost commented Apr 15, 2019

Status "Ready To Commit".

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 15, 2019
@SharkyKZ
Copy link
Contributor

This is not the right approach, IMO. The layout should not be setting user state. This way admin filter is forced whenever a page containing a module with this layout is visited.

@infograf768
Copy link
Member Author

@SharkyKZ
There are 2 possible solutions:

  1. Check if the position is cpanel and set userstate in the layout.
    This would work for all admin modules in the cpanel position and only for these

  2. The only other solution I see is to add in EACH admin module helper the code
    Factory::getApplication()->setUserState('com_modules.modules.client_id', 1);

What do you propose?

@SharkyKZ
Copy link
Contributor

SharkyKZ commented Apr 15, 2019

One option is to get client_id from form data in the field here:

$clientId = Factory::getApplication()->getUserState('com_modules.modules.client_id', 0);

$clientId  = $this->form->getValue('client_id', '', Factory::getApplication()->getUserState('com_modules.modules.client_id', 0));

Other option would be to add client_id to URL and have controller set the user state based on that.

@infograf768
Copy link
Member Author

Please make the PR yourself as I do not understand what you propose.

@ghost ghost changed the title [4.0] Solving #24575: Module position no more empty when coming from CPanel [4.0] Module position no more empty when coming from CPanel Apr 19, 2019
@SharkyKZ
Copy link
Contributor

See PR #24698 for alternative.

@infograf768
Copy link
Member Author

Closing in favour of #24698
Thanks @SharkyKZ

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 23, 2019
@infograf768 infograf768 deleted the 4.0_modules_cpanel_userstate branch April 23, 2019 07:55
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.

5 participants