EZP-31783: Fixed embedding Content with read permissions #1441
Conversation
| $contentTypeService | ||
| ); | ||
|
|
||
| $this->restrictedContentTypesIdentifiers = count($restrictedContentTypes) ? $restrictedContentTypes : null; |
There was a problem hiding this comment.
doc states that this is array, but you are assigning null here.
I Would also say that empty make intention more clear than count.
Also if possible, typehint array in more specific manner, like string[] in that case.
alongosz
changed the title
|
Side remark @SerheyDolgushev, Story pattern "As an Editor..." applies to... stories, not bugs. This is really confusing, because Stories apply to master only. |
src/lib/UniversalDiscovery/Event/Subscriber/RichTextEmbedAllowedContentTypes.php
Outdated
Show resolved
Hide resolved
Sorry, but personally I think it should go to |
|
Yes, we can merge it to 1.5 |
SerheyDolgushev
force-pushed
the
fix-udw-link-content
branch
from
1c54cff
to
b3d9ac4
Compare
|
@lserwatka rebased on |
| return []; | ||
| } | ||
|
|
||
| $restrictedContentTypes = $contentTypeService->loadContentTypeList($restrictedContentTypesIds); |
There was a problem hiding this comment.
Have you tested this with weak user with no permissions to content type/read (or class/read legacy speaking)?
There was a problem hiding this comment.
This PR is deployed on a few large projects. And no issues were reported there. But I`m not sure exactly this case was tested. I hope your QA will check all edge cases (like this).
| return array_values(array_map(function (ContentType $contentType): string { | ||
| return $contentType->identifier; | ||
| }, (array)$restrictedContentTypes)); |
There was a problem hiding this comment.
please re-format this code, it's not readable
There was a problem hiding this comment.
Sorry, can you please suggest a more readable option?
There was a problem hiding this comment.
return array_values(
array_map(static function (ContentType $contentType): string {
return $contentType->identifier;
},
iterator_to_array($restrictedContentTypes)
);
There was a problem hiding this comment.
Done: 502ce9c
|
|
||
| $restrictedContentTypes = $contentTypeService->loadContentTypeList($restrictedContentTypesIds); | ||
|
|
||
| return array_values(array_map(function (ContentType $contentType): string { |
There was a problem hiding this comment.
I don't see any reason not to:
| return array_values(array_map(function (ContentType $contentType): string { | |
| return array_values(array_map(static function (ContentType $contentType): string { |
There was a problem hiding this comment.
Done in 0e3c6cf.
|
@adamwojs could you merge it up? |
|
Done @lserwatka. Thank you for contribution @SerheyDolgushev! |
Checklist:
$ composer fix-cs)