Issue #2107455 by mparker17, ravi.shankar, yogeshmpawar, pooja saraah,...
Issue #2107455 by mparker17, ravi.shankar, yogeshmpawar, pooja saraah, joseph.olstad, claudiu.cristea, KapilV, ShaunDychko, andileco, drclaw, xjm, alexpott: Allow the $items parameter to be nullable thanks to code sniffer. Image field default value not shown when upload destination set to private file storage
Closes #2107455
Merge request reports
Activity
35 public function __construct( 36 #[Autowire(service: 'cache.default')] 37 protected readonly CacheBackendInterface $cache, 38 protected readonly EntityTypeManagerInterface $entityTypeManager, 39 protected readonly EntityRepositoryInterface $entityRepository, 40 protected readonly EntityFieldManagerInterface $entityFieldManager, 41 protected readonly AccountInterface $currentUser, 42 protected readonly ImageFactory $imageFactory, 43 protected readonly ConfigFactoryInterface $configFactory, 44 protected readonly ModuleHandlerInterface $moduleHandler, 45 ) {} 46 47 /** 48 * Implements hook_file_download(). 49 */ 50 public function __invoke($uri): array|int|null { changed this line in version 2 of the diff
155 // First, check if the default image is set on the field storage. 156 $uri_from_storage = NULL; 157 $file_uuid = $field_storages[$field_name]->getSetting('default_image')['uuid']; 158 if ($file_uuid && $file = $this->entityRepository->loadEntityByUuid('file', $file_uuid)) { 159 /** @var \Drupal\file\FileInterface $file */ 160 $uri_from_storage = $file->getFileUri(); 161 $cache_tags = Cache::mergeTags($cache_tags, $file->getCacheTags()); 162 } 163 164 foreach ($field_info['bundles'] as $bundle) { 165 $field_definition = $this->entityFieldManager->getFieldDefinitions($entity_type_id, $bundle)[$field_name]; 166 $default_uri = $uri_from_storage; 167 $file_uuid = $field_definition->getSetting('default_image')['uuid']; 168 // If the default image is overridden in the field definition, use 169 // that instead of the one set on the field storage. 170 if ($file_uuid && $file = $this->entityRepository->loadEntityByUuid('file', $file_uuid)) { -
&&is higher than=in order of operations. It also doesn't look like there is test coverage for the default image being set on the bundle field config instead of field storage. 
I don't see an issue with operator precedence here. It only affects readability, and there are no standards that require this.
Edited by kksandr-
OK, I do see this note on https://www.php.net/manual/en/language.operators.precedence.php, so TIL in PHP:
Although = has a lower precedence than most other operators, PHP will still allow expressions similar to the following: if (!$a = foo()), in which case the return value of foo() is put into $a.As mentioned in the issue, a similar conditional in https://www.drupal.org/project/drupal/issues/3403999 was problematic,
($batch = batch_get() && isset($batch['current_set'])), but apparently PHP knows that you need a variable to do the assigning.Regardless, I don't see test coverage for the default image being set on the field config with a private image. AFAICT,
Drupal\Tests\image\Functional\ImageFieldDisplayTest::testImageFieldDefaultImage()only has a test for default images set on the field storage config.
added 1 commit
added 1 commit
added 414 commits
-
f25bb0aa...b43aae5d - 391 commits from branch
project:11.x - b43aae5d...7dad069c - 13 earlier commits
- 3ec538c3 - fix coding standards
- 5f029a49 - Apply 1 suggestion(s) to 1 file(s)
- bb197dfd - Apply 1 suggestion(s) to 1 file(s)
- 0964ddf4 - Issue #2107455 by kksandr: fix standards
- 0c07681c - Issue #2107455 by kksandr: replace new service with hook class
- 69ede996 - Issue #2107455 by kksandr: replace test procedural hook with hook class
- 09cf95cf - Issue #2107455 by kksandr: fix namespace
- fa619033 - Apply 1 suggestion(s) to 1 file(s)
- e6e0661c - Issue #2107455 by kksandr: improved test coverage
- 54154739 - Fix long comment lines
Toggle commit list-
f25bb0aa...b43aae5d - 391 commits from branch
added 36 commits
-
54154739...c8a7a589 - 13 commits from branch
project:11.x - c8a7a589...70c08166 - 13 earlier commits
- 2638d01f - fix coding standards
- b43428a0 - Apply 1 suggestion(s) to 1 file(s)
- bc755dc9 - Apply 1 suggestion(s) to 1 file(s)
- 64f8c630 - Issue #2107455 by kksandr: fix standards
- fe05cfdf - Issue #2107455 by kksandr: replace new service with hook class
- 6cef5a3d - Issue #2107455 by kksandr: replace test procedural hook with hook class
- adbc6461 - Issue #2107455 by kksandr: fix namespace
- 28f9069a - Apply 1 suggestion(s) to 1 file(s)
- 970242e1 - Issue #2107455 by kksandr: improved test coverage
- 386bd83b - Fix long comment lines
Toggle commit list-
54154739...c8a7a589 - 13 commits from branch
added 43 commits
-
386bd83b...66097f2a - 20 commits from branch
project:11.x - 66097f2a...be92c0aa - 13 earlier commits
- 622d06e1 - fix coding standards
- 533f18f3 - Apply 1 suggestion(s) to 1 file(s)
- dbfc5bdd - Apply 1 suggestion(s) to 1 file(s)
- 28c39ca0 - Issue #2107455 by kksandr: fix standards
- 920b992a - Issue #2107455 by kksandr: replace new service with hook class
- 7b550365 - Issue #2107455 by kksandr: replace test procedural hook with hook class
- 65304ab1 - Issue #2107455 by kksandr: fix namespace
- 0c11a33c - Apply 1 suggestion(s) to 1 file(s)
- fb3f31a3 - Issue #2107455 by kksandr: improved test coverage
- 2a223b7e - Fix long comment lines
Toggle commit list-
386bd83b...66097f2a - 20 commits from branch