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.

Sign up for GitHub

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

Jump to bottom

Remove send mail option #1978

Merged
merged 3 commits into from
Nov 2, 2016
Merged

Remove send mail option #1978

merged 3 commits into from
Nov 2, 2016

Conversation

schiessle
Copy link
Member

Remove remaining "notify by email" and "send link by mail" code. Was already started here, but there was still some code left: #1929

As discussed here: #1916

cc @rullzer

@schiessle schiessle added the 3. to review Waiting for reviews label Nov 2, 2016
@schiessle schiessle added this to the Nextcloud 11.0 milestone Nov 2, 2016
@mention-bot
Copy link

@schiessle, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ChristophWurst, @rullzer and @PVince81 to be potential reviewers.

nickvergessen
nickvergessen previously requested changes Nov 2, 2016
View reviewed changes
apps/files/lib/Controller/ViewController.php
@@ -235,7 +235,6 @@ public function index($dir = '', $view = '', $fileid = null, $fileNotFound = fal
$params['owner'] = $storageInfo['owner'];
$params['ownerDisplayName'] = $storageInfo['ownerDisplayName'];
$params['isPublic'] = false;
$params['mailNotificationEnabled'] = $this->config->getAppValue('core', 'shareapi_allow_mail_notification', 'no');
$params['mailPublicNotificationEnabled'] = $this->config->getAppValue('core', 'shareapi_allow_public_notification', 'no');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this as well?

apps/files/templates/index.php
@@ -16,7 +16,6 @@
<input type="hidden" name="ownerDisplayName" id="ownerDisplayName" value="<?php p($_['ownerDisplayName']); ?>" />
<input type="hidden" name="fileNotFound" id="fileNotFound" value="<?php p($_['fileNotFound']); ?>" />
<?php if (!$_['isPublic']) :?>
<input type="hidden" name="mailNotificationEnabled" id="mailNotificationEnabled" value="<?php p($_['mailNotificationEnabled']) ?>" />
<input type="hidden" name="mailPublicNotificationEnabled" id="mailPublicNotificationEnabled" value="<?php p($_['mailPublicNotificationEnabled']) ?>" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this as well?

apps/files/tests/Controller/ViewControllerTest.php
@@ -218,7 +218,6 @@ public function testIndexWithRegularBrowser() {
'defaultFileSortingDirection' => 'asc',
'showHiddenFiles' => 0,
'fileNotFound' => 0,
'mailNotificationEnabled' => 'no',
'mailPublicNotificationEnabled' => 'no',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this as well?

apps/files_sharing/lib/Capabilities.php
@@ -73,8 +73,6 @@ public function getCapabilities() {
}
$res["public"] = $public;

$res['user']['send_mail'] = $this->config->getAppValue('core', 'shareapi_allow_mail_notification', 'no') === 'yes';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set to false, so a check for the capability on client is saver?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

apps/files_sharing/lib/Migration.php
* this was dropped for Nextcloud 11 in favour of share by mail
*/
public function removeSendMailOption() {
$this->config->deleteAppValue('core', 'shareapi_allow_mail_notification');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also shareapi_allow_public_notification

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

lib/private/Settings/Admin/Sharing.php
@@ -52,7 +52,6 @@ public function getForm() {
// Built-In Sharing
'allowGroupSharing' => $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes'),
'allowLinks' => $this->config->getAppValue('core', 'shareapi_allow_links', 'yes'),
'allowMailNotification' => $this->config->getAppValue('core', 'shareapi_allow_mail_notification', 'no'),
'allowPublicMailNotification' => $this->config->getAppValue('core', 'shareapi_allow_public_notification', 'no'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop

tests/lib/Settings/Admin/SharingTest.php
->willReturn('no');
$this->config
->expects($this->at(4))
->method('getAppValue')
->with('core', 'shareapi_allow_public_notification', 'no')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop

tests/lib/Settings/Admin/SharingTest.php
->willReturn('no');
$this->config
->expects($this->at(4))
->method('getAppValue')
->with('core', 'shareapi_allow_public_notification', 'no')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop

tests/lib/Settings/Admin/SharingTest.php
@@ -121,7 +116,6 @@ public function testGetFormWithoutExcludedGroups() {
[
'allowGroupSharing' => 'yes',
'allowLinks' => 'yes',
'allowMailNotification' => 'no',
'allowPublicMailNotification' => 'no',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop

tests/lib/Settings/Admin/SharingTest.php
@@ -220,7 +209,6 @@ public function testGetFormWithExcludedGroups() {
[
'allowGroupSharing' => 'yes',
'allowLinks' => 'yes',
'allowMailNotification' => 'no',
'allowPublicMailNotification' => 'no',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop

@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Nov 2, 2016
@nickvergessen nickvergessen mentioned this pull request Nov 2, 2016
Merged
@schiessle schiessle force-pushed the remove-send-mail-option branch from e016efd to 11ad9d4 Compare November 2, 2016 16:25
@schiessle schiessle added 3. to review Waiting for reviews feature: sharing and removed 2. developing Work in progress labels Nov 2, 2016
@schiessle schiessle force-pushed the remove-send-mail-option branch from 11ad9d4 to ee35e4d Compare November 2, 2016 17:27
@schiessle
remove 'send mail notification' option from sharing, replaced by send…
f556c58 
…-by-mail feature

Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@schiessle schiessle force-pushed the remove-send-mail-option branch from ee35e4d to d09216a Compare November 2, 2016 17:30
@schiessle
Copy link
Member Author

@rullzer @nickvergessen please review... everything should be fine now

@rullzer
Copy link
Member

rullzer commented Nov 2, 2016

good stuff! 👍

I updated core.json

schiessle and others added 2 commits November 2, 2016 21:17
@schiessle
remove the 'shareapi_allow_mail_notification' setting
d235c48 
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@rullzer @schiessle
Remove from core.json
7d17b42 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@schiessle schiessle force-pushed the remove-send-mail-option branch from 86b0f0b to 7d17b42 Compare November 2, 2016 20:17
@MorrisJobke
Copy link
Member

Tested and works 👍

@MorrisJobke MorrisJobke merged commit 39573e6 into master Nov 2, 2016
@MorrisJobke MorrisJobke deleted the remove-send-mail-option branch November 2, 2016 21:36
schiessle added a commit that referenced this pull request Nov 3, 2016
@schiessle
sharedialogmailview.js no longer exists, see #1978
be862ee 
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
schiessle added a commit that referenced this pull request Nov 3, 2016
@schiessle
sharedialogmailview.js no longer exists, see #1978
e5d328e 
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
schiessle added a commit that referenced this pull request Nov 3, 2016
@schiessle
sharedialogmailview.js no longer exists, see #1978
6c05fa3 
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@MorrisJobke
Copy link
Member

Slightly different, but anyways: #5897

@snyk-bot snyk-bot mentioned this pull request May 2, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rullzer rullzer rullzer left review comments

@nickvergessen nickvergessen nickvergessen left review comments

Assignees
No one assigned
Labels
3. to review Waiting for reviews feature: sharing
Projects
None yet
Milestone
Nextcloud 11.0
Development

Successfully merging this pull request may close these issues.
5 participants
@schiessle @mention-bot @rullzer @MorrisJobke @nickvergessen