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

Add option to disable the automatic updater #457

Merged
merged 3 commits into from Dec 28, 2018

Conversation

Projects
None yet
3 participants
@xh3n1
Copy link
Member

xh3n1 commented Dec 26, 2018

Add option to disable the updater
Signed-off-by: Xheni Myrtaj <myrtajxheni@gmail.com>

@xh3n1 xh3n1 requested review from samtuke and michield Dec 26, 2018

@samtuke
Copy link
Contributor

samtuke left a comment

Please also add an entry here for documentation: https://github.com/phpList/phplist3/blob/master/public_html/lists/config/config_extended.php

Once the PR is merged please add an entry here for the new constant also: https://resources.phplist.com/system/config/constants

Add docs
Signed-off-by: Xheni Myrtaj <myrtajxheni@gmail.com>

@xh3n1 xh3n1 force-pushed the disable-updater branch from e32b3e8 to 6ab50ea Dec 26, 2018

@xh3n1

This comment has been minimized.

Copy link
Member

xh3n1 commented Dec 26, 2018

@samtuke added the entry for documentation.

@michield
Copy link
Member

michield left a comment

I think in general it's fine, but it would be nice to try to improve the UX a little.

@@ -606,6 +606,10 @@
if (!defined('USERSPAGE_MAX')) {
define('USERSPAGE_MAX', 1000);
}
// if false, it will disable the automatic updater.
if (!defined ('ALLOW_UPDATER')){

This comment has been minimized.

@michield

michield Dec 26, 2018

Member

it would be nice here to verify some requirements, like file and directory permissions and then to disable it automatically when those requirements are not met. That will be a better UX, suppressing the links in the UI when they cannot be used anyway.

@samtuke

This comment has been minimized.

Copy link
Contributor

samtuke commented Dec 26, 2018

Where might the logic for those checks go? Users should to be prompted to fix permissions in order to be able to upgrade.

Update public_html/lists/config/config_extended.php
Co-Authored-By: xh3n1 <myrtajxheni@gmail.com>
@xh3n1

This comment has been minimized.

Copy link
Member

xh3n1 commented Dec 28, 2018

I think this PR is good to go, The files permission check is done on the updater and the update cannot continue without users fixing the permission of displayed files. After that they do that, they can try again.

@samtuke samtuke merged commit f9617bd into master Dec 28, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@samtuke samtuke deleted the disable-updater branch Dec 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment