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

[FEATURE-REQUEST] Config option to turn off bulk delete #291

Open
roryfahy opened this issue Sep 17, 2023 · 6 comments
Open

[FEATURE-REQUEST] Config option to turn off bulk delete #291

roryfahy opened this issue Sep 17, 2023 · 6 comments
Assignees
Labels
enhancement New feature or request

Comments

@roryfahy
Copy link
Contributor

Describe the problem you're proposing to solve
[FEATURE-REQUEST] Bulk Delete #277 introduced a bulk delete feature. The issue is that there are some resources where I'd want them to be deletable but not bulk deletable.

Describe the solution you'd like
a config opt to turn off bulk delete :bulk_delete: false

Describe alternatives you've considered
shutting off delete altogether.

Additional context
Add any other context or screenshots about the feature request here.

@roryfahy roryfahy added the enhancement New feature or request label Sep 17, 2023
@aesmail
Copy link
Owner

aesmail commented Sep 17, 2023

@roryfahy can you please provide more details about your use case? If you can.

@roryfahy
Copy link
Contributor Author

Sure @aesmail. We want to enable the business people to come in and make updates to resources like %OrgEntitlement that dictate how long any member of an Org might have premium access. In cases like this, it would be handy to allow them to delete individual records but don't want to expose a way for them to shoot themselves in the foot by accidentally selecting all of the records and pressing the delete action. As it is now, we are not going to be able to expose the individual delete unless we can avoid the bulk delete. We just consider it too dangerous. Please let me know if I can add any further detail/ I'd be happy to help out with the pr if thats wanted. Thank you for Kaffy, I really appreciate the work you're doing here 🙏

@aesmail
Copy link
Owner

aesmail commented Sep 18, 2023

@roryfahy appreciate the feedback. I'm thinking of making the authorized?/2 function more flexible. Currently, it receives the schema and the conn struct.

Making authorized? receive the context, the resource, the schema, the conn, and the action might make Kaffy way more flexible with permissions.

This might also solve your issue more flexibly. You can just define the function in the admin module and prevent bulk deletion.

This approach might not hide "delete selected records" option though.
However, I feel this might be the way to move forward.

What do you think? @roryfahy

@roryfahy
Copy link
Contributor Author

image I haven't used `authorized?/2` yet but based on the current description it seems like it will bounce a user to the dashboard if they cant access a resource, is that right? So in this case would `bulk delete` be considered the "resource"? Im having trouble understanding how this might work in practice

@aesmail
Copy link
Owner

aesmail commented Sep 18, 2023

In its current functionality, authorized?/2 wouldn't help you to achieve what you want. It would actually hide the resource (i.e. the schema wouldn't appear in the side menu). I was thinking of extending the function to accept more parameters, but maybe the better option is to add a new authorize/4 function and deprecate authorized?/2.

Let's say you have your %OrgEntitlement resource under the organizations context for example:
The authorize/4 function signature could be:
authorize(conn, context, schema, action)

When users go to the index page, this would be called:
OrgEntitlementAdmin.authorize(conn, "organizations", OrgEntitlement, :index)

When they try to bulk delete:
OrgEntitlementAdmin.authorize(conn, "organizations", OrgEntitlement, :bulk_delete)

Return values could be:

  • {:ok, conn}: the user is authorized and the request should proceed (this is the default value).
  • {:error, conn} the user is not authorized. conn is sent to the client directly.

This would give much more control to the developer.

@roryfahy
Copy link
Contributor Author

oh, yeah that would be really neat. I like that idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants