Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EZP-29833 : Limiting "Content\Create" to Section disables "Publish" button #787

Merged
merged 1 commit into from
Jan 10, 2019

Conversation

pawbuj
Copy link
Contributor

@pawbuj pawbuj commented Jan 3, 2019

Question Answer
Tickets EZP-29833
Bug fix? yes
New feature? no
BC breaks? no
Tests pass? yes
Doc needed? no
License GPL-2.0

Fixes permission check for content creation when rights are limited some section.

Checklist:

  • Coding standards ($ composer fix-cs)
  • Ready for Code Review

@pawbuj pawbuj self-assigned this Jan 3, 2019
@ezrobot
Copy link

ezrobot commented Jan 3, 2019

Tool version : PHP CS Fixer 2.7.1 Sandy Pool by Fabien Potencier and Dariusz Ruminski
Command executed php-cs-fixer --dry-run -v fix
This Pull Request does not respect PSR-2 Coding Standards, please, see the suggested diff below:

diff --git a/src/lib/Menu/ContentCreateRightSidebarBuilder.php b/src/lib/Menu/ContentCreateRightSidebarBuilder.php
index c3338b8..ba847fe 100644
--- a/src/lib/Menu/ContentCreateRightSidebarBuilder.php
+++ b/src/lib/Menu/ContentCreateRightSidebarBuilder.php
@@ -107,7 +107,6 @@ class ContentCreateRightSidebarBuilder extends AbstractBuilder implements Transl
         $canCreate = $this->permissionResolver->canUser('content', 'create', $contentCreateStruct, [$locationCreateStruct]) && $parentContentType->isContainer;
         $canPreview = $this->permissionResolver->canUser('content', 'versionread', $contentCreateStruct, [$locationCreateStruct]);
 
-
         $publishAttributes = [
             'class' => self::BTN_TRIGGER_CLASS,
             'data-click' => '#ezrepoforms_content_edit_publish',

@pawbuj pawbuj force-pushed the EZP-29833-fixes-permission-check branch from 49a2c1d to 69cd8e2 Compare January 3, 2019 13:31
@ViniTou ViniTou changed the title EZP-29833 : fixes permission check for content create action EZP-29833 : Limiting "Content\Create" to Section disables "Publish" button Jan 4, 2019
*
* @return \eZ\Publish\API\Repository\Values\Content\ContentCreateStruct
*/
private function createContentCreateStruct(Location $location, ContentType $contentType, Language $language)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return class could be also typehinted

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@ViniTou
Copy link
Contributor

ViniTou commented Jan 4, 2019

Also, not sure how all of you are feeling about creating those structs inside menuBuilder instead of passing them as options.
@webhdx
@Nattfarinn
@adamwojs

Copy link
Contributor

@mikadamczyk mikadamczyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt that it is worth to add a private method, which we can not reuse, rather than add one line. The similar situation is at least in two places.

@pawbuj
Copy link
Contributor Author

pawbuj commented Jan 4, 2019

The main reason to add createContentCreateStruct function was to increase readability of the code.

@pawbuj pawbuj force-pushed the EZP-29833-fixes-permission-check branch from 69cd8e2 to 953b454 Compare January 4, 2019 10:01
@micszo micszo self-assigned this Jan 8, 2019
@micszo
Copy link
Member

micszo commented Jan 8, 2019

@pawbuj according to Affects Version/s base needs to be changed to 1.3. Also please rebase afterwards.

Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about Content on the Fly? This place isn't covered by this fix. Could we fix that as well?

ezp-29833

@pawbuj pawbuj changed the base branch from master to 1.3 January 9, 2019 07:50
@pawbuj
Copy link
Contributor Author

pawbuj commented Jan 9, 2019

@pawbuj according to Affects Version/s base needs to be changed to 1.3. Also please rebase afterwards.

done

@pawbuj
Copy link
Contributor Author

pawbuj commented Jan 9, 2019

@micszo yes, sure we should also fix this, but I think that we should have a new task for this in jira.

@lserwatka
Copy link
Member

@pawbuj we don’t create tickets per affected version. You simply set right target branch (1.3 in your case) and after merge we merge it up to master.

@pawbuj
Copy link
Contributor Author

pawbuj commented Jan 9, 2019

I mean a new ticket for the button in the modal (screenshot by @micszo ). Target branch is already changed to 1.3.

@micszo
Copy link
Member

micszo commented Jan 10, 2019

@pawbuj there you go https://jira.ez.no/browse/EZP-30001.

Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@micszo micszo removed their assignment Jan 10, 2019
@lserwatka lserwatka merged commit a5d4bdf into 1.3 Jan 10, 2019
@lserwatka
Copy link
Member

You can merge it up

@lserwatka lserwatka deleted the EZP-29833-fixes-permission-check branch January 10, 2019 14:01
mateuszdebinski pushed a commit that referenced this pull request Feb 13, 2024
…newly created user when space button on the keyboard is used (#787)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

8 participants