-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Allow admins to request text input confirmation when deleting certain objects #13954
Comments
For me, that delete dialog should be much more detailed.. A list of all objects that will be deleted with that action, would be very handy. I once openend #8551 for that, but it was not (yet) approved. I stil find it troublesome that the deletion of a device, also deletes IP addresses, ... |
While it would be nice, I think that's a much more complex change. I image we'd need to traverse all the foreign key relationships from the object being deleted (recursively) and I'm not positive the amount of complexity of that change is justified here. I'd be happy to implement this as suggested, and if we think we need additional visibility in the future we can reconsider displaying the cascading effects. (Unless others completely disagree ofc) |
There used to be a "native" django way to do this: from django.db.utils import DEFAULT_DB_ALIAS
from django.contrib.admin.utils import NestedObjects
collector = NestedObjects(using=DEFAULT_DB_ALIAS)
collector.collect(queryset)
related_objects = collector.data Perhaps both your (@PieterL75) and OP's FR could be combined |
Let's keep these as separate FRs, as they each propose related but discrete functionality, and can easily be implemented individually. |
Fair enough, thoughts on moving forward with a more complete implementation for this? I got some free cycles, and we'd love to have this feature upstream. FWIW I'd also be happy to work on the other request once we wrap this up, I do think there's value in having it and the snippet that @jsenecal shared seems to make it pretty straightforward to implement. |
What if... also, When doing bulk deletes, it's not very handy to always have to copy/paste a message. |
That doesn't seem consistent. Unless you're deeply familiar with the Netbox object model you wouldn't really know why you're getting asked for additional confirmation on some cases vs. others.
There's already a page dedicated to bulk deletes, and we haven't run into accidental bulk deletes, which to me means that it's sufficient. -- Overall I'd say I like better your initial suggestion (show the cascading effects) but as @jeremystretch mentioned, it's probably best tackled in a separate FR. We have a real need for the improvement I'm proposing in this FR, and would love to keep it small and scoped. |
To add a bit more context on the following:
What I'm thinking is adding a new optional setting, say The idea here is that we wouldn't enforce a new behavior, and instead allow admins to select the models for which they want this additional protection. If the setting is not defined, we'd also not be requiring confirmation. We can iterate a bit on the design of this (e.g. what if I want to enforce this on all models expect X, Y and Z? Do we even want to build for that use case?), but that's the intent. |
FYI #14089 (comment) solved the issue of showing what related objects will be deleted when you delete an object. it's in the feature branch so it will be released with netbox 3.7. I would still like to see this fr happen. |
This is great, thanks a ton for the update @ITJamie - I'd still love to contribute this FR. If you're part of the team that prioritizes updates, lmk if there's anything I can do to ship this. We've ran into exactly the same problem you're describing 🙃 |
We would love to see this implement (and/or the other proposal in #8551)
We've hit this numerous times and it is a real pain. The presence of two delete buttons which will do different things is confusing alright. The proposed changes would go a long way to preventing people making this mistake. |
This honestly strikes me as a terribly abrasive disruption to the object deletion workflow. Where I've encountered similar controls in the wild, I've never considered them useful. Even for such risky operations such as deleting a GitHub repository, I've never seen a point in forcing the user to validate his or her intention in such an obtuse manner. At best, it merely trains the user to blindly copy and paste text into a form field. I can't imagine how annoying it would be to repeat this every time one goes to delete an object in NetBox. Further, NetBox v3.6 will introduce an improved deletion confirmation dialog (see #13690), listing all dependent objects to be deleted (e.g. interfaces being removed in response to deleting a device). IMO this is a far more valuable improvement and further invalidates the already questionable justification for this change.
@topranks it sounds like your issue is with the page layout then, not with the deletion dialog. I invite you to open your own FR proposing an improved layout, however bear in mind that we're planning a UI refresh for v4.0 anyway (see #12128). |
Thanks Jeremy. Yeah I could see it being an annoyance if displayed for every object deletion. It's probably only realistic if there was a way to control it so it would only appear for certain types of objects, based on user preference. The improved "Confirm Deletion" (#13690) box looks great! It definitely addresses the problem we have so I think that solution is fine.
Ok yes. I will have a think about it, one option might be to move the "Clone, Edit, Delete" buttons under the (for example on a device) 'Device' tab, so they didn't appear if one moved to the 'Interfaces' or other tabs. Thanks for the feedback. |
I'm sure you've read this part of the FR, but just as a reminder, the very first bullet point is:
I do agree that requiring confirmation for every object deletion can be disruptive, which is why I think this should be configurable. Deleting certain objects by accident in our environment can cause issues (domino effect) which is why we'd want to prompt our users for confirmation. Note that it's not the case for most object types. The improved confirmation dialog that will get released in 3.6 is great and I think it will help prevent differenty type of blunders like the ones mentioned by @topranks |
Users would simply turn it off once it (quickly) becomes annoying, defeating its purpose. For the reasons I cite above, I remain unconvinced that this feature is a worthwhile addition to NetBox, so I'm going to pass on it. Of course if it's something you really need, you always have the freedom to fork the project and make whatever changes you deem necessary to the application. |
All good, thanks for the feedback and time spent evaluating this! |
NetBox version
v3.5.9
Feature type
New functionality
Proposed functionality
As Netbox administrators we'd love to have additional an additional safety check to prevent accidental deletes. The proposed change is the following:
This is very similar to how other systems prevent accidental deletes. For example, Github requires this type of user input when archiving a project.
I implemented a quick proof of concept in my fork to show the intended workflow. This doesn't have anything to make the confirmation required on some object types only, but it's enough to get the idea across. Here's a recording of the workflow:
Screen.Recording.2023-10-03.at.7.58.17.AM.mov
It goes without saying that I'm more than glad to contribute this feature if approved. I believe this would require some frontend changes, and a bit of fiddling around with the configuration to introduce a new field.
Use case
We use Netbox across our company to keep an inventory of assets, drive automation, and configuration management. If someone accidentally deletes an object it can have side effects that are difficult to track, and undo. To prevent this, we'd like to be able to protect a little bit better some object types. We already prevent deletion of some object types using permissions and conditions, but that isn't always enough as some resources still need to be modified by many people, and the more people we have making changes, the greater the chances of blunders. This new feature would make people even more aware of the change they are making.
Database changes
None
External dependencies
None
The text was updated successfully, but these errors were encountered: