Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
In #1782244: Convert image styles $style array into ImageStyle (extends ConfigEntity), several of the procedural image functions were moved to methods, but not uri(). Instead, image_style_entity_uri() was added, had a typo, and has no coverage.
Since this function has existed for less than 24 hours, no API change.
Comment | File | Size | Author |
---|---|---|---|
#13 | 1885644-interdiff-13.txt | 7.82 KB | andypost |
#13 | 1885644-image-uri-13.patch | 13.98 KB | andypost |
#11 | 1885644-image-uri-11.patch | 13.78 KB | andypost |
#10 | 1885644-interdiff-10.patch | 13.52 KB | andypost |
#10 | 1885644-image-uri-10.patch | 14.05 KB | andypost |
Comments
Comment #2
swentel CreditAttribution: swentel commentedTests pass locally for me as well.
Comment #3
aspilicious CreditAttribution: aspilicious commentedFatal error: Call to a member function id() on a non-object in /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/image/lib/Drupal/image/Tests/ImageAdminStylesTest.php on line 37
FATAL Drupal\image\Tests\ImageAdminStylesTest: test runner returned a non-zero error code (255).
Comment #4
tim.plunkettThe testbots run in a subdir (or with a prefix, I forget) and that throws url() off.
Comment #5
xjmCan we add an inline comment here explaining the subdirectory thing?
Comment #6
andypostI'm changing a path to image style in #1809376-4: Use EntityListController for image styles
So let's commit a proper patch
Comment #7
tim.plunkettNo, this doesn't make it any harder to do that conversion, it just means you have to change one line here.
Also, this is a current major bug in HEAD, that's a conversion.
Comment #8
sunHm. I don't see any other entity type that overrides the ::uri() method in this way. The existing are specifying a uri_callback.
Could we move that approach/idea of overriding ::uri() into a separate issue? I'd find it inconsistent if some entities in core are doing it this way and some are doing it in another way.
Essentially, just s/manage/edit/ + plus test assertion here?
That said, @andypost is right in that the patch *should* be /manage instead of /edit. So I think it would make sense to perform that change first.
Comment #9
tim.plunkettView::uri() does :)
And the rest should.
Comment #10
andypostAdded changes to fix urls from #1809376-4: Use EntityListController for image styles
Comment #11
andypostThis is not a part of patch :( It seems that we have no tests for registry
Comment #12
sunThanks, that makes a lot more sense.
The /edit path/suffix should actually be a MENU_DEFAULT_LOCAL_TASK; i.e., the path/to/%image_style itself should be the primary router path.
See contact_menu() for a reference implementation.
Comment #13
andypostThis a bit of out-scope but really makes sense
Comment #14
sunThank you!
Comment #15
webchickCommitted and pushed to 8.x. Thanks!
Comment #16
andypostNext clean-up #1809376-7: Use EntityListController for image styles