Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change script loading order in worflowengine #31770

Merged
merged 1 commit into from Mar 31, 2022
Merged

Conversation

artonge
Copy link
Contributor

@artonge artonge commented Mar 31, 2022

Fix: #31709

Signed-off-by: Louis Chemineau louis@chmn.me

Fix: #31709

Signed-off-by: Louis Chemineau <louis@chmn.me>
@artonge artonge added the 3. to review Waiting for reviews label Mar 31, 2022
@artonge artonge added this to the Nextcloud 24 milestone Mar 31, 2022
@artonge artonge self-assigned this Mar 31, 2022
@artonge artonge requested review from a team, PVince81, skjnldsv and szaimen and removed request for a team March 31, 2022 08:34
@szaimen
Copy link
Contributor

szaimen commented Mar 31, 2022

@artonge I grepped for script( in our codebase and could these places potentially be broken as well?
See

click to expand
./core/templates/login.php:script('core', 'login');
./core/templates/publicshareauth.php:   script('core', 'publicshareauth');
./core/templates/recommendedapps.php:script('core', 'recommendedapps');
./core/templates/installation.php:script('core', 'install');
./core/templates/loginflow/grant.php:script('core', 'login/grant');
./core/templates/loginflow/authpicker.php:script('core', 'login/authpicker');
./core/templates/loginflowv2/grant.php:script('core', 'login/grant');
./core/templates/loginflowv2/authpicker.php:script('core', 'login/authpicker');
./apps/user_ldap/templates/renewpassword.php:script('user_ldap', 'renewPassword');
./apps/user_ldap/templates/settings.php:script('user_ldap', [
./apps/federation/templates/settings-admin.php:script('federation', 'settings-admin');
./apps/theming/templates/settings-admin.php:script('theming', 'settings-admin');
./apps/theming/templates/settings-admin.php:script('theming', '3rdparty/jscolor/jscolor');
./apps/settings/templates/settings/admin/security.php:script('settings', 'vue-settings-admin-security');
./apps/settings/templates/settings/admin/server.php:script('settings', [
./apps/settings/templates/settings/admin/delegation.php:script('settings', 'vue-settings-admin-delegation');
./apps/settings/templates/settings/frame.php:script('settings', 'settings');
./apps/settings/templates/settings/frame.php:script('core', 'setupchecks');
./apps/settings/templates/settings/frame.php:script('files', 'jquery.fileupload');
./apps/settings/templates/settings/personal/security/password.php:script('settings', [
./apps/settings/templates/settings/personal/security/password.php:      script('settings', 'security_password');
./apps/settings/templates/settings/personal/security/webauthn.php:script('settings', 'vue-settings-personal-webauthn');
./apps/settings/templates/settings/personal/security/authtokens.php:script('settings', 'vue-settings-personal-security');
./apps/settings/templates/settings/personal/personal.info.php:script('settings', [
./apps/settings/templates/settings-vue.php:script('settings', 'vue-settings-apps-users-management');
./apps/systemtags/templates/admin.php:script('core', 'systemtags');
./apps/systemtags/templates/admin.php:script('systemtags', 'admin');
./apps/twofactor_backupcodes/templates/personal.php:script('twofactor_backupcodes', 'settings');
./apps/files_sharing/templates/Settings/personal.php:script(\OCA\Files_Sharing\AppInfo\Application::APP_ID, 'personal-settings');
./apps/updatenotification/templates/admin.php:script('updatenotification', 'updatenotification');
./apps/encryption/templates/settings-admin.php:script('encryption', 'settings-admin');
./apps/encryption/templates/settings-personal.php:script('encryption', 'settings-personal');
./apps/encryption/appinfo/app.php:\OCP\Util::addscript('encryption', 'encryption');
./apps/files/templates/settings-personal.php:script(\OCA\Files\AppInfo\Application::APP_ID, 'personal-settings');
./apps/oauth2/templates/admin.php:script('oauth2', 'oauth2');
./apps/federatedfilesharing/templates/settings-admin.php:script('federatedfilesharing', 'settings-admin');
./apps/federatedfilesharing/templates/settings-personal.php:script('federatedfilesharing', 'settings-personal');
./apps/dav/templates/settings-admin-caldav.php:script('dav', 'settings-admin-caldav');
./apps/dav/templates/settings-personal-availability.php:script('dav', 'settings-personal-availability');
./apps/sharebymail/templates/settings-admin.php:script('sharebymail', 'settings-admin');
./apps/files_external/templates/settings.php:   script('files_external', [
./apps/files_external/templates/settings.php:                   script('files_external', $script);
./apps/files_external/templates/settings.php:                   script('files_external', $script);
./apps/workflowengine/lib/Listener/LoadAdditionalSettingsScriptsListener.php:           script('core', 'systemtags');
./apps/workflowengine/lib/Listener/LoadAdditionalSettingsScriptsListener.php:           script(Application::APP_ID, [
./lib/private/legacy/template/functions.php:function script($app, $file = null) {
./lib/private/legacy/template/functions.php:function vendor_script($app, $file = null) {
./build/stubs/intl.php:function locale_get_script($locale) { }
./build/stubs/intl.php:function locale_get_display_script($locale, $in_locale = null) { }
./build/stubs/redis.php:     * $sha = $redis->script('load', $script);
./build/stubs/redis.php:     * $redis->script('load', $script);
./build/stubs/redis.php:     * $redis->script('flush');
./build/stubs/redis.php:     * $redis->script('kill');
./build/stubs/redis.php:     * $redis->script('exists', $script1, [$script2, $script3, ...]);
./build/stubs/redis.php:    public function script($command, $script)
./build/stubs/redis_cluster.php:     * $sha = $redisCluster->script('load', $script);
./build/stubs/redis_cluster.php:     * $redisCluster->script(['127.0.0.1',6379], 'load', $script);
./build/stubs/redis_cluster.php:     * $redisCluster->script(['127.0.0.1',6379], 'flush');
./build/stubs/redis_cluster.php:     * $redisCluster->script(['127.0.0.1',6379], 'kill');
./build/stubs/redis_cluster.php:     * $redisCluster->script(['127.0.0.1',6379], 'exists', $script1, [$script2, $script3, ...]);
./build/stubs/redis_cluster.php:    public function script($nodeParams, $command, $script) { }
./3rdparty/aws/aws-sdk-php/src/ConnectParticipant/ConnectParticipantClient.php: * @method \Aws\Result getTranscript(array $args = [])

interesting is also this line:
./apps/encryption/appinfo/app.php:\OCP\Util::addscript('encryption', 'encryption');

@artonge
Copy link
Contributor Author

artonge commented Mar 31, 2022

@artonge I grepped for script( in our codebase and could these places potentially be broken as well?

Hopefully not, I upgraded to Util::addScript as script is deprecated, but it would have worked with script as the problem was only in the order that the scripts were added.

@szaimen
Copy link
Contributor

szaimen commented Mar 31, 2022

@artonge I grepped for script( in our codebase and could these places potentially be broken as well?

Hopefully not, I upgraded to Util::addScript as script is deprecated, but it would have worked with script as the problem was only in the order that the scripts were added.

Good to know! But this looks still wrong to me:

./apps/encryption/appinfo/app.php:\OCP\Util::addscript('encryption', 'encryption');

@artonge
Copy link
Contributor Author

artonge commented Mar 31, 2022

Good to know! But this looks still wrong to me:

First argument is the app, second one is the script, so the line targets apps/encryption/js/encryption.js

@szaimen
Copy link
Contributor

szaimen commented Mar 31, 2022

Good to know! But this looks still wrong to me:

First argument is the app, second one is the script, so the line targets apps/encryption/js/encryption.js

Yes, I understood that but it looks to me like it should be addScript instead of addscript or do both work?

@blizzz blizzz merged commit 87e079d into master Mar 31, 2022
@blizzz blizzz deleted the fix/systemtag_loading branch March 31, 2022 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: System tags workflows not working anymore
3 participants