Widget setting to allow or disallow deleting entities from the system

The code for itself looks perfect. IMHO it also makes sense to be able to not show the delete flag. Its not needed in case you just reference to it.
@jsbalsera
Do you think we should have tests for this part of configuration?

I agree, some setting like this setting is necessary.

The current logic was introduced by @bojanz in #1617886: Remove entity without deleting. It attempts to infer from the widget settings ("allow existing" and "allow new") whether or not these entities are managed outside of this context and so whether removal should entail deletion. However, the introduction of form modes in D8 makes this inference (even more) questionable.

In my case, I have 2 different IEF widgets in 2 form modes. In one, for ordinary users, I don't allow referencing existing, and in the other for advanced users I do allow referencing existing. In neither case do I want removal to equal automatic deletion, but currently in the one for ordinary users I will get it without warning or choice.

I suggest the setting should be a dropdown with title "Delete @labelplural when removed", and 3 values: "Always", "Never" and "Optionally". "Optionally" would trigger the current checkbox, "Always" and "Never" bypass the need for it. This setting should be the single source of truth about this, the settings about adding new/existing should not be involved.

Nit: Even with current patch, I think the summary text would be better as something like "Removed @label is always deleted"

TLDR; The patch is an improvement as it is, but a 3 way choice might be even better than a boolean.

Hello all,

What's needed to get this patch merged?

If it's something I can assist with I'm happy to volunteer -- we've been testing it for a few weeks with no issues, and running with the patch has allowed us to get halfway to where we want to be.

(The other half is to remove the "Edit" operation from the form -- I'm going to take a run at a separate patch, although it feels like it could come under the umbrella of this one... Happy to take advice on that.)

For context, the end goal is to present the inline entity form, allowing creation and reuse, but with no Edit operation, and no "Delete this media from the system" checkbox. We don't want to restrict user permissions, we just want to remove the options immediately presented while editing a node to avoid... accidents. (i.e. our authors can edit and delete media, but have to deliberately access the media's edit page via a different method to be able to do so.)

There's definitely a complementary user-education piece around the reuse of media assets which is driving this need!

Anyway, next steps?

J

Patch attached, with a second control to disable the Edit button on pre-existing media. (Just-created media can always still be edited.)

BTW This is my first patch submission, so hand-holding very welcome.

J

Hi @jiminary, your patch looks good but I'd suggest it should be a separate issue. Deleting entities - as opposed to just stopping to reference them - is a sufficiently serious matter that it's probably worth keeping the discussion tightly focused on it.

Hey @jonathanshaw

Good point -- new ticket created at https://www.drupal.org/node/2913571

What's the protocol for this? Delete the updated patch from this issue?

This should do it

There doesn't seem to be an outcry of support for this piece of configuration. Perhaps it is something you could implement specifically on your site using a form alter hook?

Fair enough. I guess other sites have better-behaved authors than we do. Or just smarter ones... :)

The patch works fine for now -- if it stops working I'll move it into a form alter hook and make the code available.

Cheers.

J

So this issue is about deleting the entity, not just the reference, right? In my opinion it would be nice to have a choice to delete the entity or to unlink it. Patch offered in Widget setting to allow or disallow removing existing references solves it and should be considered here.

Yeah, I agree with @pminf, I think #2875716 and this are the same issues...Should we mark it as duplicate?

I agree with #13 and #14 that this is a duplicate, so let's mark it as such.

I'm not sure they are the same, but trying to discuss them separately is hard so I'm fine to merge them.