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

Fixes ability to save Focus permissions #2707

Merged
merged 2 commits into from
Oct 12, 2016

Conversation

alanhartless
Copy link
Contributor

@alanhartless alanhartless commented Oct 10, 2016

Q A
Bug fix? y
New feature? n
Related user documentation PR URL na
Related developer documentation PR URL todo
Issues addressed (#s or URLs) #2674
BC breaks? n
Deprecations? y

Description:

Saving a role that customizes focus permissions would result in an error.

Steps to test this PR:

  1. Create a new role.
  2. Customize Focus Permissions.
  3. Save and note no error

Steps to reproduce the bug:

  1. Same as above only results in error

List deprecations along with the new alternative:

  1. Permission use of plugin:somePlugin is no longer required due to permission class changes introduced in 2.2. This means that it's also no longer required to know track plugin vs core bundle. So, 2.2.1 deprecates this to be removed in 3.0.

@alanhartless alanhartless added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test needs-documentation PR's that need documentation before they can be merged labels Oct 10, 2016
@alanhartless alanhartless added this to the 2.2.1 milestone Oct 11, 2016
@escopecz
Copy link
Sponsor Member

The error is gone and I'm able to save the Focus permissions with this PR applied 👍

@escopecz escopecz added the pending-test-confirmation PR's that require one test before they can be merged label Oct 11, 2016
Copy link
Contributor

@virlatinus virlatinus left a comment

Choose a reason for hiding this comment

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

It works!

@escopecz escopecz added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed pending-test-confirmation PR's that require one test before they can be merged ready-to-test PR's that are ready to test labels Oct 12, 2016
@dongilbert
Copy link
Member

Tested and works great. +1 Thanks @alanhartless!

@dongilbert dongilbert merged commit 66d85cf into mautic:staging Oct 12, 2016
@alanhartless alanhartless deleted the bug-2674 branch October 24, 2016 01:53
@escopecz escopecz removed ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged labels Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs needs-documentation PR's that need documentation before they can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants