JS: call init function each time a modal box is reopened

[marc-farre]

Is your feature request related to a problem? Please describe.

• Controller: return $this->renderAjax('my-modal-view');
• View: Assets::register($this);
• JS:

humhub.module('myModule.test', function (module, require, $) {
    module.initOnPjaxLoad = true;
    const init = function () {
        console.log('test');
    };
    module.export({
        init: init
    });
});

First time I open the modal box, "test" is displayed in the console.
But the second time, it is not displayed.

Describe the solution you'd like

Having something similar to module.initOnPjaxLoad = true; for view rendered with ajax. E.g.
module.initOnAjaxLoad = true;

Describe alternatives you've considered

module.initOnPjaxLoad = true; loads init function on after each renderAjax().

[yurabakhtin]

@marc-farre I am not sure some solution exists to detect JS function is called from script file which was loaded by AJAX.
But as I have understood your idea you want to call the JS function module.init() on each time when a php file is rendered which contains like <script src="myModule.test.js" /> that run the JS function humhub.module('myModule.test', ...), so if it is true then I would suggest to name new option as initOnEachLoad, i.e. module with such option will be initialized on each time when the function humhub.module(...) is called.
I hope the PR #6563 is what you need, please review and test, thanks.

[marc-farre]

Thanks very much @yurabakhtin .
This is a great solution which can be used for many use cases, because it solves the problem with the initOnPjaxLoad which call the init() function on each pages, even the one where the JS is not loaded by PHP.

Your #6563 works perfectly, but the asset extending AssetBundle needs to have force copy on (which is for development environment):

public $publishOptions = [
        'forceCopy' => true,
    ];

Would you have an idea to make this solution work with the default value 'forceCopy' => false,? I've searched a little, it seams not to be easy... Perhaps with one of these events? https://github.com/defunkt/jquery-pjax#events
Thanks!

[yurabakhtin]

but the asset extending AssetBundle needs to have force copy on (which is for development environment):

@marc-farre If you tell about humhub\components\assets\AssetBundle, did you try the property public $forceCopy = true; - https://github.com/humhub/humhub/blob/master/protected/humhub/components/assets/AssetBundle.php#L92-L95 ?

[marc-farre]

did you try the property public $forceCopy = true;

Yes (see my comment #6552 (comment)).

But the doc https://docs.humhub.org/docs/develop/javascript-index/#publish-a-module-asset tells not to use it in production: "This can be useful while developing your module, but don't forget to disable this option in official releases!"

Because with forceCopy, the JS file is loaded again into the browser.

And as by default forceCopy is set to false, developers might not understand why initOnEachLoad doesn't work.

Is there any other way to trigger with JS the PHP JS assests registering?

[yurabakhtin]

And as by default forceCopy is set to false, developers might not understand why initOnEachLoad doesn't work.

@marc-farre I think developers should use forceCopy = true during developing a JS file, and on release they should revert it to forceCopy = false. I don't know another way, I use it and yes it is not comfortable because sometimes I commit asset file with forceCopy = true that's wrong of course.
I also tried to set it through config common.php like:

return [
    'components' => [
        'assetManager' => [
            'class' => 'yii\web\AssetManager',
            'forceCopy' => true,
        ],
    ]
];

but it doesn't have any effect, so yes it would be good to find a solution to keep forceCopy = true for all assets only on developer side without touch this property in each AssetBundle file.

[marc-farre]

on release they should revert it to forceCopy = false

Yes. And then initOnEachLoad doesn't work anymore.

That's why I'm not sure we can use PR #6563 because it cannot be used in production environments.

[yurabakhtin]

Yes. And then initOnEachLoad doesn't work anymore.

Hm strange, I need to test this case.

[yurabakhtin]

@marc-farre Yes, you are right. I can suggest new solution 3c02640:

• set new js module option module.initOnAjaxLoad = true;
• also from php side you should provide what ajax url should be triggered to init the module, it should be defined as module config, e.g. in Assets file like this:

public static function register($view)
{
    $view->registerJsConfig('myModule.test', [
        'initOnAjaxUrl' => Url::to(['/controller-name/action-name', 'some-param' => 'value']),
    ]);

    return parent::register($view);
}

We have to initialize the URL from php side because it may be different with enabled/disabled pretty urls.
I have decided to use the restriction with module.config.initOnAjaxUrl because otherwise module will be initialized on any ajax request.

Probably we should allow to init module on any ajax request when module.config.initOnAjaxUrl is not defined, do you think it will be useful option?

also we can make the config as array with name initOnAjaxUrls, what do you think? or is it enough when a module is limited with single URL?

[marc-farre]

Thanks! That's a great solution, even if it makes it more complicated for developers because of the need to specify the URLs in PHP, but I don't have any better ideas.

What do you think about my commit 080f487 ?

It allows multiple URLs and avoids checking URL params (which would be a problem, e.g. with grid filters).

So now the PHP code would be:

public static function register($view)
{
    $view->registerJsConfig('myModule.test', [
        'initOnAjaxUrl' => [
            Url::to(['/controller-name/action-name']), // No param even if the loaded page have params
        ],
    ]);

    return parent::register($view);
}

[yurabakhtin]

What do you think about my commit 080f487 ?

It allows multiple URLs and avoids checking URL params (which would be a problem, e.g. with grid filters).

So now the PHP code would be:

@marc-farre yes, it is good for excluding of other params like filters, thanks for imrpoving the code.

Please note here 21b777b#diff-e58e8662213684a9f6b13971c8a8560d61b19ff2fe72fbe0b4a2abb5d82c2c08R143 you moved the checking config instance.config.initOnAjaxUrls outside $(document).on('ajaxComplete', but when I tested I had the config there as undefined because it is defined only after init module, so it why I put the if (... === instance.config.initOnAjaxUrl) inside the event also it is why I have created the additional js option instance.initOnAjaxLoad. i.e. if the instance.initOnAjaxLoad === true then initOnAjaxUrl is defined from php side as well.

I will test your commits later, but if the instance.config.initOnAjaxUrls is really defined before run event ajaxComplete then no need to keep the additional var instance.initOnAjaxLoad.

---

Also it would be good to allow have the config var initOnAjaxUrl as string and as array in order to don't defined it as array when only single url is needed.

[marc-farre]

Yes, I agree initOnAjaxLoad is useless as initOnAjaxUrls is already mandatory. So I've removed it it commit df26f8b

I've also added the possibility for initOnAjaxUrls (note the s added at the end) to be a single string.

[yurabakhtin]

@marc-farre

I've also added the possibility for initOnAjaxUrls (note the s added at the end) to be a single string.

Thanks, it works.

Yes, I agree initOnAjaxLoad is useless as initOnAjaxUrls is already mandatory. So I've removed it it commit df26f8b

Ok, but before deleting the initOnAjaxLoad I wanted to be sure the instance.config.initOnAjaxUrls is really defined/filled with proper values as expected, because on my previous time testing it wasn't, it is why I have created the additional initOnAjaxLoad which is initialized at that time.

Now I have tested the last version of this PR and I still don't find the instance.config.initOnAjaxUrls initilized, it is undefined, please review my test:

I.e. the instance.config is empty on the step when the function like humhub.module('myModule.test', ...) is called first time.

So if when we have the additional module option instance.initOnAjaxLoad === true we can use this as flag to bind the event ajaxComplete only for the module, and when the event ajaxComplete will be called there we already have the defined instance.config.initOnAjaxUrls as expected, please see what code I mean:

if (instance.initOnAjaxLoad) {
    $(document).on('ajaxComplete', function (event, jqXHR, ajaxOptions) {
        if (ajaxOptions && ajaxOptions.url) {
            var initOnAjaxUrls = instance.config.initOnAjaxUrls;
            // Allow single URL as string
            if (typeof initOnAjaxUrls === "string") {
                initOnAjaxUrls = [initOnAjaxUrls];
            }
            if (typeof initOnAjaxUrls === 'object') {
                var ajaxUrl = new URL('https://domain.tld' + ajaxOptions.url);
                // Remove all params except `r` param (in case pretty URLs are disabled)
                ajaxUrl.searchParams.forEach(function (value, name) {
                    if (name !== 'r') {
                        ajaxUrl.searchParams.delete(name);
                    }
                });
                if (initOnAjaxUrls.includes(ajaxUrl.pathname + ajaxUrl.search)) {
                    initModule(instance);
                }
            }
        }
    });
}

Do you agree this? Or are you really have the instance.config.initOnAjaxUrls initialized on your side so your module is initialized each time when you run ajax request as expected?

[marc-farre]

Thanks @yurabakhtin for the explanations.

Strangely it was working on my dev instance (the instance.config.initOnAjaxUrls was already initialized when tested in the first if, before ajaxComplete).

As you can see here instance.config.initOnAjaxUrls is available on first page load:

But as you say it may not be the case on some Humhub instances, your solution is safer.

Added in commit a4b1f87

I've also added some doc at the begining of the file.

[yurabakhtin]

@marc-farre

Added in commit a4b1f87

I've also added some doc at the begining of the file.

Thank you for this!

---

I could reproduce your case with initialized instance.config.initOnAjaxUrls on first call - I just set public $defer = false; for ny test AssetBundle class, please see what order of the JS file and config code we have:

and what order when public $defer = true;:

I.e. it doesn't work when the module file is called before the config was set by code humhub.config.set({"myModule.test": ....
It seems on bug and probably we should investigate deeply why a module file is called before the config is set, because it may create problems for developing of other modules when some config var may be required on init time.

[marc-farre]

Oh, I see, thanks for your investigations!
Now I understand. When I tested, I simply copied an asset file from the mail module.
And as you can see on PR humhub/mail#349 the defer doesn't exists because the class extends yii\web\AssetBundle instead of humhub\components\assets\AssetBundle!

we should investigate deeply why a module file is called before the config is set

Yes, could be a good idea.

[yurabakhtin]

we should investigate deeply why a module file is called before the config is set

Yes, could be a good idea.

@luke- Should we create a separate issue to investigate as I have described here #6552 (comment)?

[luke-]

@yurabakhtin As you like, I haven't followed the issues that closely now.

[yurabakhtin]

@yurabakhtin As you like, I haven't followed the issues that closely now.

@luke- Ok, I have created this https://github.com/humhub/humhub-internal/issues/104

[marc-farre]

EDIT: this comment is in the wrong issue.
See #6979 (comment)

@yurabakhtin Thanks, now results are really accurate (but not tested on other drivers)!

Tested with @all, I have a 500 error:

{
    "url": "/meta-search",
    "status": 500,
    "response": "\n<h1>Invalid database configuration</h1>\n<p><strong>SQLSTATE[42000]: Syntax error or access violation: 1064 syntax error, unexpected '@'\nThe SQL being executed was: SELECT COUNT(*) FROM `content` LEFT JOIN `contentcontainer` ON `content`.`contentcontainer_id` = `contentcontainer`.`id` LEFT JOIN `content_fulltext` ON content_fulltext.content_id=content.id LEFT JOIN `space` ON contentcontainer . pk = space . id and contentcontainer .class='humhub\\\\modules\\\\space\\\\models\\\\Space' LEFT JOIN `user` `c...",
    "textStatus": "error",
    "xhr": {
        "readyState": 4,
        "responseText": "\n<h1>Invalid database configuration</h1>\n<p><strong>SQLSTATE[42000]: Syntax error or access violation: 1064 syntax error, unexpected '@'\nThe SQL being executed was: SELECT COUNT(*) FROM `content` LEFT JOIN `contentcontainer` ON `content`.`contentcontainer_id` = `contentcontainer`.`id` LEFT JOIN `content_fulltext` ON content_fulltext.content_id=content.id LEFT JOIN `space` ON contentcontainer . pk = space . id and contentcontainer .class='humhub\\\\modules\\\\space\\\\models\\\\Space' LEFT JOIN `user` `cuser` ON contentcontainer . pk = cuser . id and contentcontainer .class='humhub\\\\modules\\\\user\\\\models\\\\User' LEFT JOIN `space_membership` ON contentcontainer . pk = space_membership . space_id and contentcontainer .class='humhub\\\\modules\\\\space\\\\models\\\\Space' and space_membership . user_id =1 WHERE (content_fulltext.content_id IS NOT NULL) AND (MATCH(content_fulltext.contents, content_fulltext.comments, content_fulltext.files) AGAINST ('+@all*' IN BOOLEAN MODE)) AND (`content`.`state`=1) AND (space.id IS NOT NULL AND ( space_membership.status=3 OR (content.visibility=1 AND space.visibility != 0) ) OR cuser.id IS NOT NULL AND (   (content.visibility = 1) OR   (content.visibility = 0 AND content.contentcontainer_id=1))OR content.created_by=1 OR content.contentcontainer_id IS NULL)</strong></p>\n<p>The following connection string was used:<br><code>mysql:host=localhost;dbname=hhcore</code></p>\n<br>\n<h2>Technical information</h2>\n<p><code>[\"yii\\\\db\\\\Exception\",\"42000\",1064,\"syntax error, unexpected '@'\"]</code></p>\n<p><pre>PDOException: SQLSTATE[42000]: Syntax error or access violation: 1064 syntax error, unexpected '@' in /var/www/hhcore/protected/vendor/yiisoft/yii2/db/Command.php:1321\nStack trace:\n#0 /var/www/hhcore/protected/vendor/yiisoft/yii2/db/Command.php(1321): PDOStatement->execute()\n#1 /var/www/hhcore/protected/vendor/yiisoft/yii2/db/Command.php(1187): yii\\db\\Command->internalExecute()\n#2 /var/www/hhcore/protected/vendor/yiisoft/yii2/db/Command.php(444): yii\\db\\Command->queryInternal()\n#3 /var/www/hhcore/protected/vendor/yiisoft/yii2/db/Query.php(498): yii\\db\\Command->queryScalar()\n#4 /var/www/hhcore/protected/vendor/yiisoft/yii2/db/ActiveQuery.php(353): yii\\db\\Query->queryScalar()\n#5 /var/www/hhcore/protected/vendor/yiisoft/yii2/db/Query.php(369): yii\\db\\ActiveQuery->queryScalar()\n#6 /var/www/hhcore/protected/humhub/modules/content/search/driver/MysqlDriver.php(123): yii\\db\\Query->count()\n#7 /var/www/hhcore/protected/humhub/modules/content/search/ContentSearchProvider.php(77): humhub\\modules\\content\\search\\driver\\MysqlDriver->search()\n#8 /var/www/hhcore/protected/humhub/services/MetaSearchService.php(60): humhub\\modules\\content\\search\\ContentSearchProvider->getResults()\n#9 [internal function]: humhub\\services\\MetaSearchService->humhub\\services\\{closure}()\n#10 /var/www/hhcore/protected/vendor/yiisoft/yii2/caching/Cache.php(609): call_user_func()\n#11 /var/www/hhcore/protected/humhub/services/MetaSearchService.php(59): yii\\caching\\Cache->getOrSet()\n#12 /var/www/hhcore/protected/humhub/widgets/MetaSearchProviderWidget.php(53): humhub\\services\\MetaSearchService->search()\n#13 /var/www/hhcore/protected/humhub/widgets/MetaSearchProviderWidget.php(36): humhub\\widgets\\MetaSearchProviderWidget->initProvider()\n#14 /var/www/hhcore/protected/vendor/yiisoft/yii2/base/BaseObject.php(110): humhub\\widgets\\MetaSearchProviderWidget->init()\n#15 [internal function]: yii\\base\\BaseObject->__construct()\n#16 /var/www/hhcore/protected/vendor/yiisoft/yii2/di/Container.php(420): ReflectionClass->newInstanceArgs()\n#17 /var/www/hhcore/protected/vendor/yiisoft/yii2/di/Container.php(171): yii\\di\\Container->build()\n#18 /var/www/hhcore/protected/vendor/yiisoft/yii2/BaseYii.php(366): yii\\di\\Container->get()\n#19 /var/www/hhcore/protected/humhub/components/Widget.php(59): yii\\BaseYii::createObject()\n#20 /var/www/hhcore/protected/humhub/controllers/MetaSearchController.php(23): humhub\\components\\Widget::widget()\n#21 [internal function]: humhub\\controllers\\MetaSearchController->actionIndex()\n#22 /var/www/hhcore/protected/vendor/yiisoft/yii2/base/InlineAction.php(58): call_user_func_array()\n#23 /var/www/hhcore/protected/vendor/yiisoft/yii2/base/Controller.php(179): yii\\base\\InlineAction->runWithParams()\n#24 /var/www/hhcore/protected/vendor/yiisoft/yii2/base/Module.php(553): yii\\base\\Controller->runAction()\n#25 /var/www/hhcore/protected/vendor/yiisoft/yii2/web/Application.php(104): yii\\base\\Module->runAction()\n#26 /var/www/hhcore/protected/vendor/yiisoft/yii2/base/Application.php(385): yii\\web\\Application->handleRequest()\n#27 /var/www/hhcore/index.php(28): yii\\base\\Application->run()\n#28 {main}</pre></p>\n",
        "status": 500,
        "statusText": "Internal Server Error"
    },
    "dataType": "html",
    "html": "\n<h1>Invalid database configuration</h1>\n<p><strong>SQLSTATE[42000]: Syntax error or access violation: 1064 syntax error, unexpected '@'\nThe SQL being executed was: SELECT COUNT(*) FROM `content` LEFT JOIN `contentcontainer` ON `content`.`contentcontainer_id` = `contentcontainer`.`id` LEFT JOIN `content_fulltext` ON content_fulltext.content_id=content.id LEFT JOIN `space` ON contentcontainer . pk = space . id and contentcontainer .class='humhub\\\\modules\\\\space\\\\models\\\\Space' LEFT JOIN `user` `c...",
    "error": {},
    "errorThrown": "Internal Server Error",
    "validationError": false
}

I also have an error with <all and >all

[yurabakhtin]

@marc-farre I think the error is related to https://github.com/humhub/humhub-internal/issues/230 because I see the server reports about error in sql query:

        MATCH(
            content_fulltext.contents,
            content_fulltext.comments,
            content_fulltext.files
        ) AGAINST('+@all*' IN BOOLEAN MODE)

It seems we should remove specialt chars from keywords and keep only letters, digits and maybe some simple signs, but probably we should allow to have only letters and digits at the beginning and end of each word before pass them into the SQL query.

[marc-farre]

@yurabakhtin FYI, I've tested all special char of an AZERTY keyboard and only these 3 were creating an error.

[yurabakhtin]

@marc-farre Fixed in the commit 308e400.

[marc-farre]

Sorry, the comments about the chars issues in the meta-search engine are not related to this issue...
Discussion about it continues here: #6979 (comment)

[luke-]

@marc-farre This can be closed?

[marc-farre]

@luke- Yes. It works fine now on HumHub 1.16.
PR for the documentation: humhub/documentation#121
Should I add this new feature to MIGRATE-DEV.md?

[marc-farre]

@luke- I've created the new PR #6996 to fix an issue:
When a JS file has module.initOnAjaxLoad = true;, if the initOnAjaxUrls contains multiple params in the URL, the init function is not triggered

@yurabakhtin FYI, in the loop https://github.com/humhub/humhub/pull/6563/files#diff-e58e8662213684a9f6b13971c8a8560d61b19ff2fe72fbe0b4a2abb5d82c2c08R178-R182, ajaxUrl.searchParams.delete(name); was removing items from the array, so not all params where looped.