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.

Sign up for GitHub

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

Jump to bottom

Refactoring user bundle #8083

Merged
merged 11 commits into from Dec 9, 2019
Merged

Refactoring user bundle #8083

merged 11 commits into from Dec 9, 2019

Conversation

escopecz
Copy link
Sponsor Member

@escopecz escopecz commented Nov 2, 2019 •
edited by npracht

Please be sure you are submitting this against the staging branch.

Q A
Bug fix?
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs) #7995
BC breaks?
Deprecations?

Description:

Refactoring for Mautic 3

Steps to test this PR:

  1. Load up this PR
  2. Test all user and permission forms
  3. Including log in and password reset forms

List deprecations along with the new alternative:

In https://docs.google.com/document/d/1lHNK8hHQtvyWlz9OtfAoLupDVb5-QunQjq-HAGlAsS4/edit#

List backwards compatibility breaks:

In https://docs.google.com/document/d/1lHNK8hHQtvyWlz9OtfAoLupDVb5-QunQjq-HAGlAsS4/edit#

@escopecz escopecz added this to the 3.0.0 milestone Nov 2, 2019
@escopecz escopecz added the code-review-needed PR's that require a code review before merging label Nov 2, 2019
@escopecz escopecz mentioned this pull request Nov 2, 2019
Closed
25 tasks
@escopecz escopecz changed the base branch from staging to m2-to-m3 November 2, 2019 17:43
@npracht npracht added Mautic 3 ready-to-test PR's that are ready to test labels Nov 3, 2019
florentpetitjean
florentpetitjean approved these changes Nov 4, 2019
View reviewed changes
Copy link

@florentpetitjean florentpetitjean left a comment •
edited

Choose a reason for hiding this comment

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

I've tested :

  • User creation and suppression
  • Login with user created
  • reset password for user created
  • Edit user created and admin
  • Role creation
  • Role edition and suppression
  • Changing permission for a role
  • Checking all permission checkboxes

All is working fine ! Good job :)

@npracht npracht removed the ready-to-test PR's that are ready to test label Nov 4, 2019
hluchas
hluchas suggested changes Nov 29, 2019
View reviewed changes
Copy link
Contributor

@hluchas hluchas left a comment

Choose a reason for hiding this comment

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

Check ball choice types config again please

app/bundles/ApiBundle/Security/Permissions/ApiPermissions.php Outdated Show resolved Hide resolved
app/bundles/CoreBundle/Security/Permissions/AbstractPermissions.php Show resolved Hide resolved
@escopecz escopecz added pending-feedback PR's and issues that are awaiting feedback from the author WIP PR's that are not ready for review and are currently in progress and removed pending-feedback PR's and issues that are awaiting feedback from the author labels Dec 2, 2019
@escopecz escopecz requested a review from hluchas December 3, 2019 13:49
@escopecz escopecz removed the WIP PR's that are not ready for review and are currently in progress label Dec 3, 2019
hluchas
hluchas suggested changes Dec 3, 2019
View reviewed changes
Copy link
Contributor

@hluchas hluchas left a comment •
edited

Choose a reason for hiding this comment

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

I discovered deprecation usage inspection in PHPStorm and it shows
Snímek obrazovky 2019-12-03 v 17 10 13

EDIT: we ar e not fixing controllers

@hluchas hluchas self-requested a review December 3, 2019 16:14
@escopecz
Copy link
Sponsor Member Author

escopecz commented Dec 3, 2019

We decided we won't deal with controllers in M3. It would be very difficult and we plan to rewrite Mautic to API-first solution. It won't generate any HTML. So it would be wasted work.

hluchas
hluchas approved these changes Dec 4, 2019
View reviewed changes
Copy link
Contributor

@hluchas hluchas left a comment

Choose a reason for hiding this comment

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

👍 Good work.
Please fix minor problems and I don't want to see it again ;-)

Tested and working

app/bundles/UserBundle/Form/Type/UserType.php Outdated Show resolved Hide resolved
app/bundles/UserBundle/Form/Type/ConfigType.php Show resolved Hide resolved
@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 code-review-needed PR's that require a code review before merging labels Dec 9, 2019
@escopecz escopecz merged commit bb91302 into mautic:m2-to-m3 Dec 9, 2019
@escopecz escopecz deleted the refactoring-user-bundle branch December 9, 2019 13:34
@hluchas hluchas mentioned this pull request Dec 9, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hluchas hluchas hluchas approved these changes

@florentpetitjean florentpetitjean florentpetitjean approved these changes

Assignees
No one assigned
Labels
ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged
Projects
None yet
Milestone
3.0.0-beta
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@escopecz @hluchas @florentpetitjean @npracht