Skip to content

[4.0] Fixing Module xtd filters#28221

Closed
infograf768 wants to merge 1 commit intojoomla:4.0-devfrom
infograf768:4.0_module_xtd
Closed

[4.0] Fixing Module xtd filters#28221
infograf768 wants to merge 1 commit intojoomla:4.0-devfrom
infograf768:4.0_module_xtd

Conversation

@infograf768
Copy link
Member

Pull Request for Issue #28220

Summary of Changes

Forcing client id to site

Testing Instructions

See #28220

Before patch

Wrong type and positions in the filters
Screen Shot 2020-03-04 at 11 05 12
Screen Shot 2020-03-04 at 11 04 40

After patch

Screen Shot 2020-03-04 at 11 50 05

Screen Shot 2020-03-04 at 11 49 57

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on eba21d4


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

1 similar comment
@jwaisner
Copy link
Member

jwaisner commented Mar 4, 2020

I have tested this item ✅ successfully on eba21d4


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

@jwaisner
Copy link
Member

jwaisner commented Mar 4, 2020

RTC


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

@joomla-cms-bot joomla-cms-bot added RTC This Pull Request is Ready To Commit and removed PR-4.0-dev labels Mar 4, 2020
@Quy Quy added the PR-4.0-dev label Mar 4, 2020
@wilsonge
Copy link
Contributor

wilsonge commented Mar 4, 2020

This isn’t the correct fix. First I’m unsure whether all modals are always displaying site modules. If this is the case then make the change here

https://github.com/infograf768/joomla-cms/blob/eba21d47cce29c2f6d1bab6d8ee23e3569851494/administrator/components/com_modules/src/Model/ModulesModel.php#L110 Removing the client site check from the if statement

@infograf768
Copy link
Member Author

I can't find another use of this modal.
the code you point to should work as it is a IF OR

		// Special case for the client id.
		if ($app->isClient('site') || $layout === 'modal')
		{
			$this->setState('client_id', 0);
			$clientId = 0;
		}

but it does not... thus why I used this solution.

@infograf768
Copy link
Member Author

Folks, if I trust my last tests here, this is no more an issue.
Could someone confirm?

@brianteeman
Copy link
Contributor

confirmed

@infograf768
Copy link
Member Author

infograf768 commented Mar 6, 2020

In fact, we still have the issue if the user has first displayed the Admin modules manager.
In that case the client id is admin.
Thanks @SharkyKZ for finding out

@brianteeman
Copy link
Contributor

ah that makes sense

@infograf768
Copy link
Member Author

This is not working anymore. Closing

@infograf768 infograf768 closed this Mar 6, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 6, 2020
@SharkyKZ
Copy link
Contributor

SharkyKZ commented Mar 6, 2020

I would suggest doing this similarly to #24698 so we don't have to use user state in field class.

@infograf768
Copy link
Member Author

@SharkyKZ
Good idea.

@infograf768 infograf768 deleted the 4.0_module_xtd branch March 6, 2020 13:22
@infograf768
Copy link
Member Author

Please test new PR #28259

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.

7 participants