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

feat: Disable Sharing globally (#20318) [backport] #20500

Merged

Conversation

marination
Copy link
Collaborator

@marination marination commented Mar 29, 2023

Port of #20318

Docs: https://docs.erpnext.com/docs/v13/user/manual/en/setting-up/settings/system-settings/edit-wiki?wiki_page_patch=301c81b67e

The following is tested on a v13 site and the screenshots belong to the same:

Enable or Disable document sharing system wide

  1. Checkbox in System Settings which is unchecked by default to maintain old behaviour
    Screenshot 2023-03-29 at 1 05 46 PM

  2. If enabled, system wide, no user will have share access to any document

  3. The share button will be read only, no action (no share access). Although users can see who the doc is shared with (consistent with old behaviour)
    2023-03-29 12 58 54

  4. On invoking the share API without ignore_share_permissions (from the browser), a perm error is thrown
    2023-03-29 12 57 24

  5. If any share API is invoked with ignore_share_permissions it will go through (consistent with old behaviour). [Creating a user, doc must be shared with the user themselves]
    2023-03-29 13 08 53

  6. assign_to is handled as a special case, where assigning a doc to a user will block the assignment if the user does not have access already. The assignee is expected to have access to the document otherwise assignments can be misused
    Screenshot 2023-03-29 at 12 59 55 PM

* feat: Disable Sharing globally

- Checkbox in System Settings
- If disabled, avoid share UI render
- Share APIs return None (non-obstructing) if share APIs are invoked

* feat: Settings checkbox must toggle share permission globally

- Treat feature like a perm toggler. Essentially noone is allowed to explicity share anything
- Implicit sharing via `ignore_share_permissions` is allowed. Devs can decide where sharing should happen under the hood
- UI is made read only and not hidden. Users must see who doc is already shared with
- Make sure perm APIs used by share feature return false if sharing is disabled
- Rename checkbox to `Disable Document Sharing`

* test: (server side) Impact of disabling sharing on APIs

- Also, fix missed system setting rename in `assign_to`

* fix: Inform assigner if assignee lacks perms and sharing is disabled

- misc: readable conditions

* fix: throw instead of msgprint

* fix: Typo and appropriate message for `throw`

---------

Co-authored-by: Ankush Menat <ankush@frappe.io>
@marination marination marked this pull request as ready for review March 29, 2023 08:01
@marination marination requested review from a team and shariquerik and removed request for a team March 29, 2023 08:01
We throw instead of showing warning now
@marination marination removed the request for review from shariquerik March 29, 2023 08:29
[skip ci]
@ankush ankush merged commit 758873c into frappe:version-13-hotfix Mar 29, 2023
frappe-pr-bot pushed a commit that referenced this pull request Apr 4, 2023
# [13.52.0](v13.51.3...v13.52.0) (2023-04-04)

### Bug Fixes

* better error logging while sending email ([#20570](#20570)) ([8ae4d55](8ae4d55))
* cannot restore cancelled document if workflow is active ([6206dc0](6206dc0))
* escape HTML instead of sanitizing ([68ad9cf](68ad9cf))
* fix address query for postgres ([616dd8b](616dd8b)), closes [/github.com//pull/20537#ref-pullrequest-1645575433](https://github.com//github.com/frappe/frappe/pull/20537/issues/ref-pullrequest-1645575433)
* get workflow_state_fieldname instead of setting workflow_state to none ([3249aa2](3249aa2))
* removed unnecessary usage of `[@frappe](https://github.com/frappe).whitelist` ([#20503](#20503)) ([#20562](#20562)) ([08398cb](08398cb))
* rewrite query for postgres ([#20557](#20557)) ([#20558](#20558)) ([e400c5f](e400c5f))

### Features

* Disable Sharing globally ([#20318](#20318)) [backport] ([#20500](#20500)) ([758873c](758873c))
* make workflow state translatable ([#20412](#20412)) ([#20501](#20501)) ([1545fbd](1545fbd))

### Performance Improvements

* Faster address query with explicit joins (backport [#20537](#20537)) ([#20539](#20539)) ([955a723](955a723))
@frappe-pr-bot
Copy link
Collaborator

🎉 This PR is included in version 13.52.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants