Skip to content

Share email to ocs#1801

Closed
rullzer wants to merge 2 commits intomasterfrom
share_email_to_OCS
Closed

Share email to ocs#1801
rullzer wants to merge 2 commits intomasterfrom
share_email_to_OCS

Conversation

@rullzer
Copy link
Member

@rullzer rullzer commented Oct 19, 2016

Fixes #1277

  • Adds a new endpoint to send email for a link share
  • Removed the possibility to notify recipients of a share
    • They can configure themselfs if they want notifications
    • Long term plan is still to also accept local shares
    • Would require adding a new endpoint that we'd have to support for long time

Currently we use the mailer as is. Which is not really testable. But I want a working version in sooner rather than later.

CC: @MorrisJobke @nickvergessen @LukasReschke @icewind1991

@rullzer rullzer added this to the Nextcloud 11.0 milestone Oct 19, 2016
@mention-bot
Copy link

mention-bot commented Oct 19, 2016

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

@rullzer
Copy link
Member Author

rullzer commented Oct 19, 2016

Also it seems the autocomplete for e-mail addresses is broken. So I disabled it for now. @nickvergessen we can't just search by e-mail on the sharee api right?

@MorrisJobke
Copy link
Member

MorrisJobke commented Oct 19, 2016

@nickvergessen we can't just search by e-mail on the sharee api right?

I don't think so, because the "email address style" is actually a remote share, right? cc @schiessle

@nickvergessen
Copy link
Member

nickvergessen commented Oct 20, 2016

@MorrisJobke it just sends the link from the link share field.

We can't just search by e-mail on the sharee api right?

we can make it do so. As far as I can see it just looks into the contacts app for remote users atm.

@rullzer
Copy link
Member Author

rullzer commented Oct 22, 2016

@nickvergessen yeah it looks trough all the contacts for e-mail completion it seems. Would be nice if we could add that. I guess we could abuse the SHARE_TYPE_EMAIL for that. Bjorn might also use that in his share by email PR. But searching there is the same I'd say.

@rullzer rullzer force-pushed the share_email_to_OCS branch from 92788cc to 5ce02d1 Compare October 22, 2016 11:55
@jancborchardt
Copy link
Member

jancborchardt commented Oct 23, 2016

Removed the possibility to notify recipients of a share
They can configure themselves if they want notifications

Yeah! Thanks a lot @rullzer :)

@rullzer rullzer mentioned this pull request Oct 24, 2016
Merged
MorrisJobke
MorrisJobke requested changes Oct 24, 2016
View reviewed changes
core/js/shareitemmodel.js Outdated
dataType: 'json'
}). fail(function(xhr) {
// FIXME: a model should not show dialogs
OC.dialogs.alert(t('core', result.data.message), t('core', 'Warning'));
Copy link
Member

@MorrisJobke MorrisJobke Oct 24, 2016

Choose a reason for hiding this comment

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

Where is the result coming from?

core/js/shareitemmodel.js Outdated
}
);
dataType: 'json'
}). fail(function(xhr) {
Copy link
Member

@MorrisJobke MorrisJobke Oct 24, 2016

Choose a reason for hiding this comment

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

Maybe the xhr should be named result? 😉

Copy link
Member Author

@rullzer rullzer Oct 25, 2016

Choose a reason for hiding this comment

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

yes copy paste error

@rullzer rullzer added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Oct 25, 2016
@rullzer rullzer force-pushed the share_email_to_OCS branch from 5ce02d1 to 578ac48 Compare October 25, 2016 13:33
@rullzer rullzer added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 25, 2016
@codecov-io
Copy link

codecov-io commented Oct 25, 2016 •
edited
Loading

Current coverage is 57.78% (diff: 0.00%)

Merging #1801 into master will increase coverage by 0.26%

@@             master      #1801   diff @@
==========================================
  Files          1076       1076          
  Lines         61306      61854   +548   
  Methods        6871       7020   +149   
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          35264      35742   +478   
- Misses        26042      26112    +70   
  Partials          0          0

Sunburst

Diff Coverage File Path
0% apps/files_sharing/appinfo/routes.php
0% .../files_sharing/lib/Controller/ShareAPIController.php
•••••••••• 100% settings/templates/admin/sharing.php
•••••••••• 100% lib/private/Share/MailNotifications.php

Powered by Codecov. Last update 8957436...578ac48

@rullzer
Remove notify recipient option
57e6125 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer
Remove internal share mail function
b7046d3 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer rullzer force-pushed the share_email_to_OCS branch from 578ac48 to b7046d3 Compare October 26, 2016 18:54
@rullzer
Copy link
Member Author

rullzer commented Oct 26, 2016

Ok lets close this in favor of #1916

@rullzer rullzer closed this Oct 26, 2016
@LukasReschke LukasReschke deleted the share_email_to_OCS branch October 27, 2016 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MorrisJobke MorrisJobke MorrisJobke requested changes

Assignees

No one assigned

Labels

3. to review Waiting for reviews bug feature: sharing

Projects

None yet

Milestone

Nextcloud 11.0

Development

Successfully merging this pull request may close these issues.

6 participants

@rullzer @mention-bot @MorrisJobke @nickvergessen @jancborchardt @codecov-io