generate correct path for owner and use the display name instead of t…

[schiessle]

Use the display name if we send a share by mail instead of the user id.

Tested it with local users and different display names, but maybe @blizzz can double check it with LDAP.

@sycophantic maybe you can test the patch too and report back? Thanks!

fix #3032

[sycophantic]

@schiessle
Pulls the correct data, but for some reason it's hitting the check for the share on behalf.

[sycophantic]

Move to below $ownerDisplayName = $this->userManager->get($owner)->getDisplayName();
The variable needs to be set before the if statement, with $initiatorDisplayName not set createMailBody generates on the behalf of email.

[blizzz]

get() can return null if the user is not available. We fell into this trap already, better to have an additional check here. Same few lines later.

[schiessle]

@blizzz @sycophantic both comments addressed, please have another look. Thanks!

[sycophantic]

@schiessle
I tested in our LDAP environment and both emails are correct now.

[schiessle]

@blizzz can you also have a final look? Thanks!

[tri2000]

I'm seeing the same thing. Can you assist in resolving this? (nextCloud newbie)

[schiessle]

@tri2000 with our without this patch. This patch should fix it and will be released with Nextcloud 12 and probably the next bugfix release of Nextcloud 11.

[nickvergessen]

I would prefer to skip the owner in this case and just mention the initiator, because that user is who is known by the recipient: initiator shared »file« with you

Opinions? @jancborchardt

[nickvergessen]

That would also be inline with current reshares, activities and the sharing tab.

Only the "owner" in the file list is the owner and not the initiator. I think that could also be changed. In most cases people don't really need to know who owns the files, but whom they are working together with (initiator)

[schiessle]

After thinking about it, I agree. The owner of a file is a technical detail and a information which doesn't add any value for the recipient. I think we should drop the differentiation and just always mention the initiator, also in the file listing in case of internal shares.

[jancborchardt]

Yep, agree too – for consistency but also for familiarity as @nickvergessen mentioned. Good call!

[schiessle]

let's merge this as a bug fix and discuss the rest here: #3079

[tri2000]

Is there a quick way to resolve this on my installation prior to the patch release?

[nickvergessen]

You can fInd red lines and replace them with green lines?

[tri2000]

Did that, but still got the same result.

…

On Mon, Jan 16, 2017 at 8:55 AM, Joas Schilling ***@***.***> wrote: You can fInd red lines and replace them with green lines? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3033 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AX8h0I9ulSzxqPHXWYpwAXtMeVd6Nz2Mks5rS4TtgaJpZM4Lg1p_> .

-- Success is the progressive realization of a worthy goal or ideal.- Earl Nightingale

[tri2000]

This is what I have:
protected function sendMailNotification($filename, $link, $owner, $initiator, $shareWith) {
if ($owner === $initiator) {
$subject = (string)$this->l->t('%s shared »%s« with you', array($initiator, $filename));
} else {
$subject = (string)$this->l->t('%s shared »%s« with you on behalf of %s', array($ownerDisplayName, $filename, $initiatorDisplayName));
}

It appears to use the first option; not dropping to the else option.

[nickvergessen]

You need to add the 4 new green lines as well

[tri2000]

Thanks! That resolved it!