-
Notifications
You must be signed in to change notification settings - Fork 96
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
API Convert SecurityAdmin to be a ModelAdmin #1359
API Convert SecurityAdmin to be a ModelAdmin #1359
Conversation
1025700
to
40f1124
Compare
Unit tests failing because symfony 6 PRs need to be merged. Re-run tests after that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels a bit suspect that there's no javascript or css being removed - is there really no double-up of javascript or css for the old SecurityAdmin that is now handled by existing ModelAdmin javascript? E.g. inside client/src/legacy/SecurityAdmin.js
or client/src/styles/legacy/_SecurityAdmin.scss
?
b79bdea
to
b669289
Compare
b669289
to
9ba43b6
Compare
Co-authored-by: Maxime Rainville <[email protected]>
9ba43b6
to
48108c1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works great locally and the code makes sense, happy to merge.
Issue #1350
Continued from where Max got to with #963 - I've included him as a co-author in the commit message