Closed marc-farre closed 6 months ago
@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 https://github.com/humhub/humhub/pull/6563 is what you need, please review and test, thanks.
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 https://github.com/humhub/humhub/pull/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!
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 ?
did you try the property
public $forceCopy = true;
Yes (see my comment https://github.com/humhub/humhub/issues/6552#issuecomment-1717053985).
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?
And as by default
forceCopy
is set to false, developers might not understand whyinitOnEachLoad
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.
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 https://github.com/humhub/humhub/pull/6563 because it cannot be used in production environments.
Yes. And then
initOnEachLoad
doesn't work anymore.
Hm strange, I need to test this case.
@marc-farre Yes, you are right. I can suggest new solution https://github.com/humhub/humhub/pull/6563/commits/3c02640da2eb71621b7e2b560fd82337e690afd7:
module.initOnAjaxLoad = true;
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?
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 https://github.com/humhub/humhub/pull/6563/commits/080f487cba339f33b7ed6496efba3de70652f20f ?
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);
}
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 https://github.com/humhub/humhub/pull/6563/commits/21b777b71b1981a91c71417159a18b6fb873a065#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.
Yes, I agree initOnAjaxLoad
is useless as initOnAjaxUrls
is already mandatory. So I've removed it it commit https://github.com/humhub/humhub/pull/6563/commits/df26f8b10e3375ec64d48fc2a8c7ca53f323dbc7
I've also added the possibility for initOnAjaxUrls
(note the s
added at the end) to be a single string.
@marc-farre
I've also added the possibility for
initOnAjaxUrls
(note thes
added at the end) to be a single string.
Thanks, it works.
Yes, I agree
initOnAjaxLoad
is useless asinitOnAjaxUrls
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?
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 https://github.com/humhub/humhub/pull/6563/commits/a4b1f87f2db75af5e108fce247f4aa5852aff6e9
I've also added some doc at the begining of the file.
@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.
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 https://github.com/humhub/mail/pull/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.
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 https://github.com/humhub/humhub/issues/6552#issuecomment-1719534940?
@yurabakhtin As you like, I haven't followed the issues that closely now.
@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
EDIT: this comment is in the wrong issue. See https://github.com/humhub/humhub/pull/6979#issuecomment-2096290940
@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
@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.
@yurabakhtin FYI, I've tested all special char of an AZERTY keyboard and only these 3 were creating an error.
@marc-farre Fixed in the commit https://github.com/humhub/humhub/pull/6979/commits/308e40014a2c7014b9a21b92b2c15f3205fa7b6e.
Sorry, the comments about the chars issues in the meta-search engine are not related to this issue... Discussion about it continues here: https://github.com/humhub/humhub/pull/6979#issuecomment-2096290940
@marc-farre This can be closed?
@luke- Yes. It works fine now on HumHub 1.16. PR for the documentation: https://github.com/humhub/documentation/pull/121 Should I add this new feature to MIGRATE-DEV.md?
@luke- I've created the new PR https://github.com/humhub/humhub/pull/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.
Is your feature request related to a problem? Please describe.
return $this->renderAjax('my-modal-view');
Assets::register($this);
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;
loadsinit
function on after eachrenderAjax()
.