Disabled page caching

HI @droprocker,

I have update your patch by passing services with dependency injection in recently_read.recently_read_subscriber service(event subscriber).
Please review.

Created an issue fork, applied the patch from #4, and changed a few things.

So first thing I noticed with the implementation, it only worked for entity types of node, this is in my opinion not the correct functionality of the module since in the configs you can enable recently read for any entity type. Also checking entity type from the path is an incorrect approach, someone could be using pathauto (or similar modules) to modify URL structure. So now I first get the ids of all enabled entity types in the read type config and load the entity if it exists in requests attributes.

Returned that switch statement from the original implementation. Again looking at how it was implemented before, if no bundle was selected in the config that means that every entity of that type should be added to recently read.

Also for now disabled the authentication check, and commented out the code for now. Again this is my opinion/interpretation, that recently read, as is, should also work for anonymous users. But maybe a new feature request issue should be added, to add a checkbox to the configuration form, where you can select to enable authentication check.

General code cleanup removed old code from the .modules file and added a few doc comments.

Locally tests pass, it would be nice if someone reviewed the changes.

I removed the switch as it did not make sense to me. As the allowedTypes does not return true or false. I added the condition if allowedTypes are empty (so not set) to also go ahead and insert entity (if the module intention is actually if nothing is set, set it for all). Also removed the static calls for loading entities and used entity type manager to do that.
I also left in the commented out code, but think it should be removed. The issue for anonymous user can be dealt in the separate issue, that already exists, like @admirlju mentioned.
The only I would say kinda major issue is (not sure if this issue introduced it or if it was always present) is that if you decide to remove entity type from the configuration, you get an error on the page (The website encountered an unexpected error. Please try again later.). From logs it seems it's connected to the view block.
Notice: Undefined index: base in Drupal\views\Plugin\views\query\QueryPluginBase->getEntityTableInfo() (line 297 of /core/modules/views/src/Plugin/views/query/QueryPluginBase.php)

InvalidArgumentException: A valid cache entry key is required. Use getAll() to get all table data. in Drupal\views\ViewsData->get() (line 140 of /core/modules/views/src/ViewsData.php).

So maybe this can get merged (after somebody confirms the caching actually works) and then a new issue needs to be created to deal with the issue mentioned above?

It looks like that major issue is present from before, so I opened an issue for it 🐛 The website encountered an unexpected error Needs review .

There was already an open issue for this ✨ Write the visit in Kernel TERMINATE event Needs review . With one difference, this version does the saving before the requested data is returned to the client, while the other issue handles it after the client receives the data.

Testing both fixes, the data gets cached correctly, so that part works for both. But the module functions a bit differently, in this fix the client sees the current entity on the list as soon as the page loads while on the other they see it after leaving the entity. But because of that, the time needed to display the content to the client is faster in the other fix since it does all the extra processing after the client receives the data.

I prefer the other fix since page loading time will always be faster, but if the preferred functionality is to display the current entity as soon as it's loaded, then this is the correct fix. For that reason, I'm leaving both fixes as Needs Review.

Sadly Drupal (as of 8th Sept 2023) doesn't have event listeners. If they did we could have implemented both solutions and the admin could select one or the other.