-
Notifications
You must be signed in to change notification settings - Fork 32
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
enable user hooks only for internal backend #95
Conversation
Codecov Report
@@ Coverage Diff @@
## master #95 +/- ##
=========================================
Coverage 81.71% 81.71%
+ Complexity 278 277 -1
=========================================
Files 31 31
Lines 1083 1083
=========================================
Hits 885 885
Misses 198 198
Continue to review full report at Codecov.
|
Signed-off-by: LEDfan <[email protected]>
@@ -52,7 +53,7 @@ public function process(IEntry $entry) | |||
$config = \OC::$server->getConfig(); | |||
$xmppPreferMail = $config->getAppValue('ojsxc', 'xmppPreferMail', false); | |||
$xmppPreferMail = $xmppPreferMail === true || $xmppPreferMail === 'true'; | |||
$serverType = $config->getAppValue('ojsxc', 'serverType', 'external'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sualko This was not intentional I guess? (That the default is external)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw I added another commit which substitutes all server type string literals with constants.
@sualko AFAIK the default the backend is internal, but this wasn't used in the check. I added a static method to ensure it's the same in the whole code base. |
for backward compatibility with php < 7.1
@sualko I just fixed an incompatibility with NC 14 (https://travis-ci.org/nextcloud/jsxc.nextcloud/jobs/350202403#L570) |
This was already fixed in #96 |
I will remove your commit and merge master with a forced push. |
.... I just discovered that, removed my commit again.. Sorry 🙈 |
fix #91