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.

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

[QA] notification bell does not appear for pending group shares #1192

Closed
jnweiger opened this issue Aug 16, 2023 · 4 comments
Closed

[QA] notification bell does not appear for pending group shares #1192

jnweiger opened this issue Aug 16, 2023 · 4 comments
Labels

Comments

@jnweiger
Copy link
Contributor

jnweiger commented Aug 16, 2023

Seen with core 10.13.0-beta1 and activity 2.7.2and both core 10.13.0-beta1 and 10.12.2 (not a regression)

  • admin disables automatic share accept in the personal sharing settings.
  • admin creates user alice
  • alice logs in in a separate broser session, creates a folder adoc, and shares with group admin
  • admin refreshes the browser, no notification bell appears. BAD
  • admin reviews the activity tab, and sees the notification there. OK

  • alice shares with user admin, then the bell appears
    image

Expected behaviour: Both user and group shares behave the same.

@pako81
Copy link

pako81 commented Aug 16, 2023

Not reproducible on 10.13.0-beta1 with both activity 2.7.1 and 2.7.2:

image

@pako81
Copy link

pako81 commented Aug 16, 2023

Reproducible when the option Automatically accept new incoming local user shares is opted out by the single user rather than by the admin itself for all the users.

@pako81
Copy link

pako81 commented Aug 17, 2023

Problem is the following:

  • Given the following group test group having user1 and user2 as members
  • And given the admin did not modify the option Automatically accept new incoming local user shares, which is by default enabled
  • And given user1 opted this option out in his/her personal settings page (section sharing), while user2 did not
  • When user admin shares folder for test group with this group, this is how the oc_share table looks like:
MariaDB [owncloud10130activity]> select * from oc_share;
+----+------------+------------+-----------+---------------+--------+-----------+-------------+-------------+-------------+-----------------+-------------+------------+----------+------------+-------+-----------+------------+------------+
| id | share_type | share_with | uid_owner | uid_initiator | parent | item_type | item_source | item_target | file_source | file_target     | permissions | stime      | accepted | expiration | token | mail_send | share_name | attributes |
+----+------------+------------+-----------+---------------+--------+-----------+-------------+-------------+-------------+-----------------+-------------+------------+----------+------------+-------+-----------+------------+------------+
| 57 |          1 | test group | admin     | admin         |   NULL | folder    | 196         | NULL        |         196 | /for test group |          31 | 1692255908 |        0 | NULL       | NULL  |         0 | NULL       | NULL       |
| 58 |          2 | user1      | admin     | admin         |     57 | folder    | 196         | NULL        |         196 | /for test group |          31 | 1692255908 |        1 | NULL       | NULL  |         0 | NULL       | NULL       |
+----+------------+------------+-----------+---------------+--------+-----------+-------------+-------------+-------------+-----------------+-------------+------------+----------+------------+-------+-----------+------------+------------+
2 rows in set (0.000 sec)
  • a new subshare entry will be created for every user who opted out (user1 only in this case) having status accepted equal to 1, which means a pending share, while the parent group share has accepted set to 0, which means accepted
  • when logging in user1 will indeed see this share as pending in his/her Shared with you section but no notification bells will appear.

This is done by the following code https://github.com/owncloud/core/blob/master/apps/files_sharing/lib/Controller/Share20OcsController.php#L590-L600, which checks if the share is a group share and if $globalAutoAccept is set, and for every user having !$userAutoAccept it creates a subshare with status pending.

Problem is that there the parent group share is not set to status pending as well, so we end up in https://github.com/owncloud/core/blob/master/apps/files_sharing/lib/Service/NotificationPublisher.php#L79-L81 and no notification will be created.

Even if we set the status to pending for the parent group share in Share20OcsController.php, https://github.com/owncloud/core/blob/master/apps/files_sharing/lib/Service/NotificationPublisher.php#L52-L61 will create a notification for all group members, even those who chose to automatically accept shares...

Not sure this can be fixed. Maybe @jvillafanez @phil-davis @IljaN you see a solution here.

@jvillafanez
Copy link
Member

I don't know if it's possible, but maybe the state of the group share should be dynamically evaluated based on the subshares: if there is at least a pending subshare, the group share should also be pending. The additional problem is that the notification is sent based on the share not on the the subshares.

My main worry is that the change could break more things, and I'm not sure how much we need to test

@IljaN IljaN closed this as completed Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants