Problem/Motivation

If we have a list view e.g. the content view, which is being cached then the hook for adding the operations will not work properly, because a Break Lock operation is added only if the entity is currently locked, which is a dynamic behaviour, but the view might be already cached with or without the break lock operation, which will not change until the cache is invalidated, because if the view is retrieved from the cache then the hook will not be invoked.

Proposed resolution

Three options:
Completely remove the operation and offer a controller instead of a view for all content entity types where the entities of all entity types having a lock are listed.
Always add the operation and if on clicking on it there is no lock then show a message that there is no lock.
On adding or removing a lock invalidate the entity_type_list cache tag.

I think that the first approach would be more user friendly than the second and third one actually means there is no need in caching the view at all.

Remaining tasks

User interface changes

API changes

Data model changes

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hchonov created an issue. See original summary.

chr.fritsch’s picture

I think this is not only a problem with the entity operations. Content lock also offers some view fields, for e.g. owner of the current lock. Adding/ removing a lock, doesn't invalidate this fields, so the view is then out of date as well.

oknate’s picture

Here's a patch that invalidates the cache tag for the entity and for the entity list for that entity's entity type.

This fixed the issue for me on a view where unlocking or locking wasn't updating the view.

oknate’s picture

Status: Active » Needs review
quiron’s picture

I had both issues (the one of the issue description and the one described by @chr.fritsch) and the patch fixes both. Thanks!

rodrigoaguilera’s picture

Status: Needs review » Reviewed & tested by the community

The patch looks good to me. Maybe it doesn't cover every case where the list is cached but at least solves the most obvious one.

vuil’s picture

Version: 8.x-1.x-dev » 8.x-2.x-dev
Status: Reviewed & tested by the community » Needs work

And, what about the 8.x-2.x-dev branch?

volkerk’s picture

Flushing the entity_list tag might not be a good idea since this will also affect frontend. Maybe flushing only the view / view result cache of the view, the action was triggered from, would be a more sensible approach. That imho would have to be done from views bulk operation form.

greenSkin’s picture

Status: Needs work » Reviewed & tested by the community

Working in 2.x for me.

solideogloria’s picture

Status: Reviewed & tested by the community » Needs work

Patch fails to apply in both branches in tests.

LRoels’s picture

Status: Needs work » Reviewed & tested by the community

Patch applies on version 2.3.0 and fixes the issue stated above.

Comment in #8 should still be taken into account but marking this as reviewed since it does work and fixes the issue.
Up to the maintainer to see if they want to implement it this way or not.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

I think given #8 we need to come up with a better approach.

alexpott’s picture

Something like the operation adds some other cache tag to the view that we can then use to clear when we need to. This list cache tag is way too generic.

smustgrave made their first commit to this issue’s fork.

smustgrave’s picture

Status: Needs work » Needs review

added 'content_lock:' to make it more unique.

alexpott’s picture

alexpott’s picture

Status: Needs review » Needs work

@smustgrave the new cache tage should not be specific for an entity ID and we should not clear the entity list cache list here.

Also need to add the cache tag to the break lock operation so that views that use this have the new cache tag in them and are cleared when necessary.