Repeated file_usages when listing them

quietone → credited Sutharsan → .

I tested on Drupal 10.1.x, standard install and this is still relevant.

have closed 🐛 Multiple file usage records Closed: duplicate as a duplicate and adding credit.

There was a discussion around two approaches in the issue. One patch was for 8.1.x and another one was for 8.2.x

The one that was for 8.1.x #24 #30 #45 was updating a views configuration and adding aggregation.
I think in #42it was discussed that this is not the good approach. I quite agree with that.
It is good to remove un-necessary join rather than adding aggregation.

Now patches for 8.2,x, 8.6.x mentioned in #30and #45 comes to picture. I feel that this is the correct approach.
Prefereed solution:
Remove the additional join happening in hook_views_data of the FileViewsData.php class.

The test cases added in #45 seem a bit confusing, so I've removed that test case and added a new simple one to validate the error. Attached test-only patch as well. Since we are not updating any views this time, I think we won't need any update hook or upgrade path test.
I've updated issue summary accordingly focussing approach #2 which is about removing additional join in the hook_views_data.

#59 re-roll is missing a couple of changes mentioned in the #45
Reportedly FileUsageTest.php file's changes are not included in the re-roll.

The last submitted patch, 63: 2632372-63.patch, failed testing. View results →

The last submitted patch, 63: test-only-2632372-63.patch, failed testing. View results →

Opps, putting back the views related changes.
Didn't realised that those changes might break the view's functional tests.
Not uploading the test only patch since test-only patch is failing for the new test case only.

Confirmed the issue using the snippet in the issue summary and the patch in #67 solves the issue.
#63 shows the tests fail as expected.

The last submitted patch, 67: 2632372-67.patch, failed testing. View results →

I think previous failures were un-related.
Triggering run again for 11.x
Let's see how goes 🤞

The last submitted patch, 67: 2632372-67.patch, failed testing. View results →

Tried to fix the issue.
To fix the issue, I had to update the view.
Which leads me to think that can there be any regression on the existing views because of the change?
Do we need to consider any upgrade path here?

Looking forward to see opinions from other folks.

The last submitted patch, 73: 2632372-73.patch, failed testing. View results →

Updating another view which was causing issues in relationship.
Hiding the patch in #73.

This was previously RTBC and since the update needed was for a test view think it's safe to go back to RTBC!

Re: #73, an upgrade path should not be necessary as long as the views being changed are only used for testing. At the moment, there are no changes to any views config, so an upgrade path is not necessary.

I manually tested this, and it worked as designed, without any updates to existing config.

It did however require a cache clear to work.

Should we add an empty post update hook to trigger the cache invalidation that resets the views config?

Or should we leave it as is knowing that the missing cache clear doesn't break anything, its just the bug doesn't get fixed until it is cleared

If the standard is to do an empty post_update hook think it wouldn't be terrible to add here.

Added an empty post_update hook.

The last submitted patch, 80: 2632372-80.patch, failed testing. View results →

There is a literally random failure on this patch, but it seems to work. I just don't understand the "why": Removing the built-in relationship between file_usage and file_managed (which seems like a useful thing to define) only to add this relationship back in the view itself?

If it really does make sense to remove this from the views data, then the above comment should be updated too:

Provide field-type-things to several base tables; on the core files table ("file_managed") so that we can create relationships from files to entities, and then on each core entity type base table so that we can provide general relationships between entities and files.

Removing this from views data seems problematic if every custom view that relied on it needs to be updated, so it presumably would need a change record?

By the way, I noticed that media was never added here as one of the core entity type base tables..