Drupal 10 Compatibility from Upgrade Status

[rosiel]

GitHub Issue: Islandora/documentation#2235

• Other Relevant Links (Google Groups discussion, related pull requests,
  Release pull requests, etc.)

• follows Get tests passing #957 and Apply Drupal Rector to Islandora. #943 in getting Islandora 10-comptaible.

What does this Pull Request do?

Removes deprecated code and updates compatiblity declarations.

What's new?

• it's ->getMainResponse() now.

• alphabetize the dependency list

• Remove D8 and add D10 to compatibility

• Remove shims that allowed deprecated code to be called (pre-D8 compatibility?)

• add access checks to entity queries..

• Does this change add any new dependencies? No

• Does this change require any other modifications to be made to the repository
  (i.e. Regeneration activity, etc.)? No

• Could this change impact execution of existing code? No

How should this be tested?

• Do the tests run on D10?
• Can you install islandora on D10 and run it with no problems?

Documentation Status

• Does this change existing behaviour that's currently documented? eventually we will update the docs with this minor change, but probably once we update the islandora starter site.
• Does this change require new pages or sections of documentation?
• Who does this need to be documented for?
• Associated documentation pull request(s): ___ or documentation issue ___

Additional Notes:

Any additional information that you think would be helpful when reviewing this
PR.

Interested parties

Tag (@ mention) interested parties or, if unsure, @Islandora/committers

[rosiel]

Oh no, more errors!

The 9.5.x failures are because I accidentally created a 3.0.0 release without specifying drupal/core:^10. That's been fixed, sorry for the missing tag.

The kernel tests - I would love any help with this. I can't figure out why functions that we didn't write seem to be returning the wrong types. It's not failing on my local setup, so it must be from a different combination of dependency versions.

[whikloj]

@rosiel GuzzleHttp\Psr7\MessageTrait has a signature with a return type of array (docs), but our tests are returning a string.

So changing from

$head_prophecy->getHeader('Link')
      ->willReturn('<some-path-to-a-tombstone>; rel="hasTombstone"');

to

$head_prophecy->getHeader('Link')
      ->willReturn(['<some-path-to-a-tombstone>; rel="hasTombstone"']);

might be all it needs.

https://github.com/rosiel/islandora/blob/drupal10/tests/src/Kernel/FedoraAdapterTest.php#L237-L238
https://github.com/rosiel/islandora/blob/drupal10/tests/src/Kernel/FedoraAdapterTest.php#L265-L266

[rosiel]

Thanks! I didn't realize where that came from! This prophesy thing is still a little bit of a black box to me.

[rosiel]

This is interesting! From Drupal 10.1:

There was 1 error:

1) Drupal\Tests\islandora\Functional\DeleteMediaTest::testDeleteMediaAndFile
Behat\Mink\Exception\ResponseTextException: The text "Deleted 2 items." was not found anywhere in the text of the current page.

Sure enough I go to a drupal 10.1 box and delete a single media and its single file, and I get "Deleted 3 items".

It looks like something gained entity status between drupal 10.0 and 10.1. I'll continue to investigate.

[rosiel]

I found it! The introduction of a new pair of permissions in Drupal 10.1 is causing "Delete media and file" to return "Deleted 1 item" and "2 items have not been deleted because you do not have the necessary permissions."

One problem can be solved by adding the 'delete own files' or 'delete any files' permission to the test, but I think that would cause tests to fail on all Drupals < 10.1.

The other problem is that 3 items are at play. I'm not sure if this was the case prior to Drupal 10. It seems intuitive to me that only 2 items (1 media and 1 file) are intended to be deleted by this Action.

[rosiel]

Option 1

When looking for files to delete, skip whatever's in the build-in media thumbnail field. If the same file is the thumbnail and the source field (as in the case in images) then that file would already be marked for deleted by virtue of being in the source field.

This would move us farther from a path of potential integration between our thumbnails and drupal media thumbnails.

Option 2

We check each file to see if its $file->getFilename() matches one of ["generic.png", "audio.png", "video.png", "no-thumbnail.png"] and don't mark it for deletion if that's the case.

This seems a little more fragile and opens possibilities for real files not getting deleted by virtue of having a wrong filename.

Other options?

...

[alxp]

The tests all run correctly for me except the ones that give unable to connect to stomp broker errors since I"m not running on the full Islandora stack.

Just update the Features version requirement and this should be good.