Skip to content

Fix email shares not being shown to other users - #32719

Merged
st3iny merged 1 commit into
masterfrom
fix/noid/email-shares-not-shown
Jun 20, 2022
Merged

Fix email shares not being shown to other users#32719
st3iny merged 1 commit into
masterfrom
fix/noid/email-shares-not-shown

Conversation

@st3iny

@st3inyst3iny commented Jun 3, 2022

Copy link
Copy Markdown
Member

I adapted the logic from

if ($reshares === false) {
$qb->andWhere($qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId)));
} else {
if ($node === null) {
$qb->andWhere(
$qb->expr()->orX(
$qb->expr()->eq('uid_owner', $qb->createNamedParameter($userId)),
$qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId))
)
);
}
}

How to reproduce?

  • Install groupfolders app.
  • Have at least two users in the same group.
  • Create groupfolder with some subfolders.
  • Share subfolder by mail to external mail address.

Expected behaviour:

Share by mail is shown to other users in the same group. Sharing sub folders of groupfolders with users works like this too.

Current behaviour

Share is only shown to the admin who created the share.

Screenshots

BeforeAfter
Screenshot 2022-06-03 at 15-32-17 groupfolder - Files - NextcloudScreenshot 2022-06-03 at 15-31-51 groupfolder - Files - Nextcloud

@st3inyst3iny added this to the Nextcloud 25 milestone Jun 3, 2022
@st3inyst3iny self-assigned this Jun 3, 2022
Comment threadapps/sharebymail/lib/ShareByMailProvider.php Fixed
@st3iny
st3inyforce-pushed the fix/noid/email-shares-not-shown branch from 6d1f36f to 46f8090CompareJune 3, 2022 14:32
@st3iny

Copy link
Copy Markdown
MemberAuthor

/backport to stable24

@st3iny

Copy link
Copy Markdown
MemberAuthor

/backport to stable23

@st3iny

Copy link
Copy Markdown
MemberAuthor

/backport to stable22

@st3iny
st3iny marked this pull request as ready for review June 7, 2022 07:37
@st3inyst3iny added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 7, 2022
@st3iny
st3iny requested review from a team, ChristophWurst, come-nc, icewind1991 and skjnldsv and removed request for a teamJune 7, 2022 07:39
@st3iny

This comment was marked as resolved.

Comment threadapps/sharebymail/lib/ShareByMailProvider.php Outdated
Comment threadtests/lib/Share20/ShareByMailProviderTest.php Outdated
@PVince81

Copy link
Copy Markdown
Member

@st3iny please squash

@st3iny
st3inyforce-pushed the fix/noid/email-shares-not-shown branch from 49c380e to 2d8319dCompareJune 10, 2022 13:26
@PVince81PVince81 added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 10, 2022
@PVince81

Copy link
Copy Markdown
Member

suspicious failure:

/src/Exception/RequestException.php","Line":113,"CustomMessage":"error while sending notification for federated share: Client error: `POST http://localhost:8080/index.php/ocm/notifications` resulted in a `400 Bad request` response:\n{\"message\":\"Can not find share with ID: 15\"}\n"}}

Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
@st3iny
st3inyforce-pushed the fix/noid/email-shares-not-shown branch from 2d8319d to c1f054aCompareJune 17, 2022 15:31
@st3iny

Copy link
Copy Markdown
MemberAuthor

suspicious failure:

/src/Exception/RequestException.php","Line":113,"CustomMessage":"error while sending notification for federated share: Client error: `POST http://localhost:8080/index.php/ocm/notifications` resulted in a `400 Bad request` response:\n{\"message\":\"Can not find share with ID: 15\"}\n"}}

That seems like a false alarm. This error also exists in other drone runs. I rebased to latest master to fix the failing scenario. I ran the integration test locally and it did not fail after rebasing.

@st3iny
st3iny merged commit c19ec67 into masterJun 20, 2022
@st3iny
st3iny deleted the fix/noid/email-shares-not-shown branch June 20, 2022 11:24
@st3iny

Copy link
Copy Markdown
MemberAuthor

/backport to stable23

@st3iny

Copy link
Copy Markdown
MemberAuthor

/backport to stable22

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

Labels

4. to releaseReady to be released and/or waiting for tests to finishbugfeature: sharing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@st3iny@PVince81@icewind1991@CarlSchwan@github-advanced-security@come-nc