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

WIP: Add OgRoleManager service #298

Closed
wants to merge 12 commits into from

Conversation

dakala
Copy link
Collaborator

@dakala dakala commented Aug 7, 2016

First attempt at issue #295. At the moment a couple of the services injected in GroupManager.php look redundant to me.

GroupManagerTest still broken from the UI. Passing with php core/scripts/run-tests.sh --color --verbose --class "Drupal\Tests\og\Unit\GroupManagerTest

Thanks everyone!

dakala added 10 commits June 16, 2016 18:38
Update from original - 201606161837
* upstream/8.x-1.x: (30 commits)
  Rename method, this actually returns test permissions, not operations.
  rename getUserMemberships to getMemberships in Og.php and related tests
  Greenify tests.
  Revert "Add temporary workaround for a core bug that causes our 8.2.x tests to fail."
  Don't pass the group content bundle IDs to .
  Add temporary workaround for a core bug that causes our 8.2.x tests to fail.
  Update documentation.
  Add PermissionManager::getDefaultPermissions() to the interface.
  We now have a PermissionManagerInterface.
  Add documentation.
  Rename method to be consistent with its calling method.
  Add missing use statement.
  Rename method and update documentation to clarify its purpose.
  Remove OgRole::getDefaultRoleProperties() to reduce confusion with GroupManager::getDefaultRoles().
  Fix circular dependency between GroupManager and PermissionManager.
  Let PermissionManager handle the retrieval and filtering of permissions.
  We're now working with full OgRole objects rather than passing property arrays around.
  Now with more green.
  Let the event subscriber create the OgRole entities.
  Fix review remarks.
  ...
* upstream/8.x-1.x: (35 commits)
  Rename [g|s]etRoles() to [g|s]etApplicableRoles().
  Update documentation.
  Change ownership from a string to a boolean.
  Remove unused use statements.
  Add unit tests for the Permission classes.
  Add missing functionality as discovered by unit testing.
  Complete test coverage of the PermissionEvent.
  Update test, the entity operation permissions are not assigned to any roles by default now.
  Do not grant permission to do any entity operation on nodes by default, to match how this is handled in D7.
  Fix typo.
  Test that the permissions specific to the Node entity are set correctly.
  Update documentation.
  Fix typo.
  Add methods to retrieve and manage group content operation permissions by their identifying properties.
  Provide an example implementation that generates the permissions for nodes.
  Provide a method to retrieve group content operation permissions from the event listeners.
  Provide a dedicated method to retrieve the full set of permissions from the event listeners.
  Move the methods to generate generic entity operation permissions to the event subscriber.
  Green is the new black.
  Update tests.
  ...
* upstream/8.x-1.x: (154 commits)
  Improve comment
  Simplify function, and add test
  Allow re-save OgRole
  Revert "Add OgRoleInterface::getID"
  Remove empty check on methods in OgRole
  Add OgRoleInterface::getID
  Code style fixes
  Add Og::getRole
  Swap arguments
  Change Og::getMembership arguments order
  Remove unused use statement.
  Move method below save
  Sniffer fixes
  Updated use classes.
  Fix phpcs errors.
  Adapt to latest DX changes.
  Correct documentation.
  Remove superfluous inline type hint.
  Sniffer fixes
  Remove field name
  ...
* upstream/8.x-1.x:
  Use the new helper method
  Sniffer fixes
  Fix test
  Add field module to tests to satisfy config
  Add request field to OG membership
  Sniffer fixes
  Role should reset OgAccess cache on save and delete
  Reset OgAccess cache
  Add test [skip ci]
  Check permissions for non-member
  Start adding tests
  Add createMembership helper function
  Fix OgMembershipReferenceItemListTest tests
  Fix sniffer
  Fix failing test
  Start fixing test [skip ci]
  Fix tests
  Fix wrong copy paste
  Add more validation to membership creation
* upstream/8.x-1.x: (194 commits)
  Fix PHP_CodeSniffer warnings.
  Revert "Provide a convenient method OgRole::loadByGroupAndName()."
  Blocked users should not be granted any permissions.
  Remove unused variable.
  Add documentation.
  Leverage OgAccess::userAccess() instead of duplicating half its logic and ignoring the other half.
  Fix PHP CodeSniffer warnings.
  Provide a convenient method OgRole::loadByGroupAndName().
  Add a test that proves that it is possible to grant permissions to non-members.
  Remove unused trait.
  Fix PHP CodeSniffer warning.
  Convert the functional access test into a kernel test.
  We don't care if a group is new when doing access checks.
  Blocked users should not have any permissions.
  Move the generation of the 'non-member membership' to the calling side.
  Update the membership state field definition, it has changed from integer to string values.
  Fix PHP CodeSniffer warning.
  Test that blocked users cannot create, update or delete any group content.
  Small documentation update.
  Change the membership state constants to strings.
  ...
@dakala dakala changed the title 295 ogrolemanager service WIP: 295 ogrolemanager service Aug 7, 2016
@dakala dakala changed the title WIP: 295 ogrolemanager service WIP: Add OgRoleManager service Aug 7, 2016
@amitaibu
Copy link
Owner

Hi @dakala, what's the status here?

@dakala
Copy link
Collaborator Author

dakala commented Aug 15, 2016

Sorry, it's the tests @amitaibu. Will take a look again and update issue with the status tomorrow

@amitaibu
Copy link
Owner

Great, thanks.

On Aug 15, 2016 4:21 PM, "Deji Akala" notifications@github.com wrote:

Sorry, it's the tests @amitaibu https://github.com/amitaibu. Will take
a look again and update issue with the status tomorrow


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#298 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAHrCxpFkDRuIJEdIgiwbFF6llYvEUVrks5qgGfkgaJpZM4JeiOA
.

@dakala
Copy link
Collaborator Author

dakala commented Aug 16, 2016

Hi @amitaibu, @pfrenssen! Who can assist with my broken unit tests please? I think it's the mocking of config factory that's causing the problem in testAddGroupNew() and testRemoveGroup(). If someone can help or point in the right direction, that'd be great. Thanks!

@amitaibu
Copy link
Owner

Sure, I can have a look. Would be great if you could fix the PHPCS errors first (most of them can be fixed automatically with PHPCBF).

After that please ping, and I'll test it. I'm on vacation, so it might take me a few days to answer, though :)

@dakala
Copy link
Collaborator Author

dakala commented Aug 17, 2016

@amitaibu thanks. I had to do the CS fixes manually - phpstorm had let me down :-) Enjoy your holiday!

@amitaibu
Copy link
Owner

Closed in favor of #145. I'll fix the tests there.

@amitaibu amitaibu closed this Aug 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants