? ? Pending

User tests: Successful: Unsuccessful:

avatar SharkyKZ
SharkyKZ
16 Jul 2020

Closes #29948.

Summary of Changes

Removes mod_multilangstatus tampering from mod_user.

Testing Instructions

Create and publish a Multilingual Status module.

Actual result BEFORE applying this Pull Request

Module created but keeps getting unpublished on every page load.

Expected result AFTER applying this Pull Request

Module created and published.

Documentation Changes Required

Joomla\Module\Multilangstatus\Administrator\Helper\MultilangstatusAdminHelper class is removed. If it's too late for that, it can be deprecated instead.

avatar SharkyKZ SharkyKZ - open - 16 Jul 2020
avatar SharkyKZ SharkyKZ - change - 16 Jul 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 16 Jul 2020
Category Modules Administration
avatar SharkyKZ SharkyKZ - change - 16 Jul 2020
The description was changed
avatar SharkyKZ SharkyKZ - edited - 16 Jul 2020
avatar SharkyKZ SharkyKZ - change - 16 Jul 2020
The description was changed
avatar SharkyKZ SharkyKZ - edited - 16 Jul 2020
avatar SharkyKZ SharkyKZ - change - 16 Jul 2020
The description was changed
avatar SharkyKZ SharkyKZ - edited - 16 Jul 2020
avatar SharkyKZ SharkyKZ - change - 16 Jul 2020
Labels Added: ?
avatar infograf768
infograf768 - comment - 16 Jul 2020

As far as I understand, this breaks the automatic display of the module when multilang is ON.
I believe this was a real improvement in J4 for multilingual sites.

avatar SharkyKZ
SharkyKZ - comment - 16 Jul 2020

Yes, it stops unpublished modules being automatically published.

avatar infograf768
infograf768 - comment - 16 Jul 2020

I can't be in favor of that. As I said, this is definitely an improvement for all multilingual sites.

avatar SharkyKZ
SharkyKZ - comment - 16 Jul 2020

Hardcode it into the template if you want. Just stop messing with user data.

avatar infograf768
infograf768 - comment - 16 Jul 2020

We have enough explaining all the time in the forums how to use that module first when someone has a multilingual issue.

I was not the one who originally created that code and I don't know how to hard code it in atum.
This basically means that, if you want to get rid of it from mod_user, I am fine with it, but please provide also the solution to hardcode it in atum.

avatar brianteeman
brianteeman - comment - 16 Jul 2020

I agree with @infograf768 Dont just remove functionality without providing a replacement

avatar SharkyKZ
SharkyKZ - comment - 16 Jul 2020

Personally, I don't think there is a need for replacement for this. If you know how to enable Language Filter plugin, publishing the module shouldn't be that hard. But OK, some options:

  1. Have the module published on all new sites.
  2. Have the module published when installing multilingual sample data.
  3. Have the module published when enabling Language Filter plugin.
  4. Have the module embedded in the template.

Take your pick. Anything is better than what we have now.

bug2

avatar Bakual
Bakual - comment - 16 Jul 2020

Why don't we have the module enabled by default, but display nothing when multilang is disabled? We have such behavior regulary in modules. Then it would automatically appear as soon as multilang is enabled. That would be the best approach imho.

A module should never automatically change its state. If it's unpublished, it has to stay unpublished, if it's published, it has to stay published. Also modules shouldn't interact with other modules, that's just plain wrong.
It should also not be hardcoded into the template, that would be equally wrong.

avatar Bakual
Bakual - comment - 16 Jul 2020

Just for reference a PR which I did last year. Turns out the code was just moved in between and the PR mistakenly closed.
#24341
It's more or less exactly the same as this one here.

avatar Bakual
Bakual - comment - 16 Jul 2020

@SharkyKZ If you add the following lines to administrator/modules/mod_multilangstatus/mod_multilangstatus.php at line 13 (after the use statement), then you got option 1 but the module doesn't show up until the language filter plugin is enabled. Similar to other modules that don't show up if there is nothing to show.

use Joomla\CMS\Language\Multilanguage;

if (!Multilanguage::isEnabled())
{
	return;
}
avatar joomla-cms-bot joomla-cms-bot - change - 16 Jul 2020
Category Modules Administration Modules Administration SQL Installation Postgresql
avatar SharkyKZ
SharkyKZ - comment - 16 Jul 2020

PR updated. Using the check from com_languages.

avatar infograf768
infograf768 - comment - 17 Jul 2020

I don't understand anything to your changes. They look much more complex than our suggestion. Any reason to choose such a code?
if you insist on this, please correct typo $mutilanguageEnabled to $multilanguageEnabled

avatar SharkyKZ
SharkyKZ - comment - 17 Jul 2020

It's the same but with dependencies passed to Multilanguage::isEnabled(). Typo fixed.

avatar infograf768
infograf768 - comment - 17 Jul 2020

Do you mean that we should change everywhere Multilanguage::isEnabled() to Multilanguage::isEnabled($app, Factory::getContainer()->get(DatabaseInterface::class)); ?

A simple grep gives me 134 occurrences of that code present in 88 files in J4

avatar SharkyKZ
SharkyKZ - comment - 17 Jul 2020

That seems to be the way going forward. But there's no need to rush replacing every existing instance now.

avatar richard67
richard67 - comment - 17 Jul 2020

@wilsonge Any opinion on the class removal (or deprecation) mentioned in the "Documentation Changes Required" section of the description?

avatar infograf768
infograf768 - comment - 17 Jul 2020

IMHO, as it does not exist in J3, no need to deprecate it.
People who are using beta versions should only use them for tests.

avatar vijaykhollam vijaykhollam - test_item - 7 Nov 2020 - Tested successfully
avatar vijaykhollam
vijaykhollam - comment - 7 Nov 2020

I have tested this item successfully on f2ff1d4


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

avatar Kaustubharas Kaustubharas - test_item - 7 Nov 2020 - Tested successfully
avatar Kaustubharas
Kaustubharas - comment - 7 Nov 2020

I have tested this item successfully on f2ff1d4


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

avatar alikon alikon - change - 7 Nov 2020
Status Pending Ready to Commit
avatar alikon
alikon - comment - 7 Nov 2020

RTC


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

avatar HLeithner HLeithner - change - 7 Nov 2020
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-11-07 12:29:49
Closed_By HLeithner
Labels Added: ?
avatar HLeithner HLeithner - close - 7 Nov 2020
avatar HLeithner HLeithner - merge - 7 Nov 2020
avatar HLeithner
HLeithner - comment - 7 Nov 2020

Thanks

Add a Comment

Login with GitHub to post a comment