8000 fix: reset workspace by barredterra · Pull Request #18795 · frappe/frappe · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: reset workspace #18795

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

Merged
merged 3 commits into from
Nov 10, 2022
Merged

Conversation

barredterra
Copy link
Collaborator

Problems:

  • get_custom_workspace_for_user might create a new Workspace object and reset_customization tries to delete it before it's even inserted
  • The page parameter was not type checked
  • Users might not have permission to delete a Workspace (for example, the owner of a customized workspace might be Administrator, so if_owner permission doesn't help)

All of these are fixed by this PR.

This method doesn't exist in future versions, hence this PR is only for version-13-hotfix.

@barredterra barredterra requested review from a team and phot0n and removed request for a team November 7, 2022 16:06
@shariquerik shariquerik merged commit 2720aed into frappe:version-13-hotfix Nov 10, 2022
@ankush
Copy link
Member
ankush commented Nov 10, 2022

@shariquerik all checks were failing because of unsupported syntax. Why merge in such cases?

image

edit: fixed here 0c0b20b

@barredterra barredterra deleted the reset-workspace branch November 10, 2022 08:36
@barredterra
Copy link
Collaborator Author

@ankush sorry! I had the false belief that v13 required python 3.8+ 🙈

@frappe-pr-bot
Copy link
Collaborator

🎉 This PR is included in version 13.44.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 Nov 30, 2022
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.

4 participants
0