-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Add detailed project management and entity handling #15327
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
Conversation
- Create ProjectEntityLoaderService for automatic entity detection via Doctrine metadata - Enhance project detail page to display all associated entities (emails, campaigns, forms, etc.) - Add translation keys for user-friendly "Project items" terminology - Implement static cache for performance optimization - Display only entity types that contain actual items - Use clean UI with collapsible panels for each entity type
Update projects detail page with latest changes from 7.x branch.
Major improvements to ProjectBundle architecture: - Implement EntityTypeConfig DTO pattern for better type safety - Convert static cache to instance-based for improved design - Refactor permission methods to use DTO instead of raw models - Merge duplicate controller actions (addEntityAction/addEntitySubmitAction) - Remove unnecessary method_exists checks in controller - Fix model mapping issues (company -> lead.company, contact -> lead) - Use model getEntity() instead of direct repository access - Sort entity types by translated labels for better UX - Optimize service by removing redundant code patterns - Improve project details page with comprehensive entity management - Add entities count column to project list with proper sorting - Implement modal-based entity selection with filtered lookup - Enhance UI with proper Bootstrap styling and layout improvements All permission checks now use EntityTypeConfig DTO pattern, providing better encapsulation and type safety throughout the bundle.
- Resolved merge conflict in ProjectBundle messages.ini by combining translations - Includes latest upstream changes and improvements
- Changed from 'mini' to 'clickable' tile type in entity selection modal - Mini type doesn't support attributes needed for AJAX modal functionality - Clickable type supports attributes while maintaining visual consistency - Preserves data-toggle, data-target, and data-header attributes for modal
- Remove model alias from services.php (no longer needed with autowiring) - Remove console.log statements from project-select-box.js
- Remove complex entity count sorting logic from controller - Change entities count column from sortable to static display - Keep entity count calculation for display purposes - Improves performance and code maintainability
- Restore mautic.project.model.project alias for backward compatibility - Ensures existing code depending on this service alias continues to work
- Refactor ProjectEntityLoaderService to eliminate repository pattern duplication - Update ProjectModel with cleaner permission handling methods - Optimize ProjectListEntityType form for better performance
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 7.x #15327 +/- ##
============================================
+ Coverage 66.71% 66.73% +0.01%
- Complexity 35848 35915 +67
============================================
Files 2329 2333 +4
Lines 144095 144507 +412
============================================
+ Hits 96140 96433 +293
- Misses 47955 48074 +119
🚀 New features to boost your workflow:
|
…issues - Fix findModelForEntityType return type to FormModel for better type safety - Remove ProjectEntityLoaderService constructor injection from AjaxController - Use ProjectModel directly in AjaxController for consistency with interface
- Change default values from string to int for limit and start parameters - Fixes PHPStan argument.type errors for InputBag::get() method
- Add proper type hints for all parameters - Add return type annotation array<int|string, string> - Fixes PHPStan missingType.parameter and missingType.iterableValue errors
- Remove $options parameter from getLookupResults method to match interface signature - Update AjaxController to not pass $options parameter - Update ProjectEntityLoaderService to not expect $options parameter - Fixes PHP Fatal error about interface method compatibility
- Remove projectId variable that was no longer being used - Maintain clean method signature after interface compatibility fix
- Fix AjaxController InputBag parameter types - Add proper type annotations to ProjectModel::getLookupResults - Fix ProjectEntityLoaderService return type assertion
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.
Thanks for pushing the code for this folks!
Before I go through and suggest multiple changes, can I get some insight into why the term 'entities' is being used? This seems to be a very developer-oriented term, and something like 'resources' might be better for the marketer side of things when it comes to understanding what is being referred to.
Was there any user research done on what to call these things? Or any research on how other tools name things? I know we did have a lot of discussions about projects v folders etc but not sure if much thought was given to this part.
@RCheesley, once I built it, I used "items" to use it like "Add items to the project". But I noticed that in another part of the project bundle, we use "entities," so I went with entities. Anyway, once we merge this, we can create a refactor PR to rename it—once we decide on the change. Fro my research items was the best option. |
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 looks great! I found some places that can be slightly improved.
I like the idea how to find the entities that can have a relationship with Project. I'm not sure about the way we get models for those. But I can't think of anything better without greater refactoring.
app/bundles/ProjectBundle/Service/ProjectEntityLoaderService.php
Outdated
Show resolved
Hide resolved
- Add entity type validation in addEntityAction GET handler - Return error message when invalid entity type is provided - Add translation for invalid entity type error message - Update functional tests to follow Mautic's standard form submission patterns - Fix testAddEntityActionWithInvalidEntityType to verify error handling - Remove deprecated projectRepository property from tests All tests passing (9 tests, 19 assertions) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
0297a4d
to
f77c885
Compare
- Add empty entityType check in AjaxController getLookupChoiceListAction - Refactor ProjectController addEntityAction to use early returns instead of nested if statements - Remove unnecessary variable assignment in ProjectModel getLookupResults method
- Move all public methods above private methods - Follows PHP coding standards for method visibility ordering
62579ba
to
59b4869
Compare
Co-authored-by: John Linhart <jan@linhart.email>
I tried make it autodiscovery to avoid make static list or something, then it need only small normalization for some parts in app/bundles/ProjectBundle/Service/ProjectEntityLoaderService.php https://github.com/mautic/mautic/pull/15327/files#diff-7581476e9d125cdd9761d1b4517a96fea4c558dcf994afca1b4ee4239c37fe56 |
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.
@kuzmany I found several small issues that I flagged in my previous review that were marked as resolved but the issue is still present.
Co-authored-by: John Linhart <jan@linhart.email>
Co-authored-by: John Linhart <jan@linhart.email>
Co-authored-by: John Linhart <jan@linhart.email>
Co-authored-by: John Linhart <jan@linhart.email>
Co-authored-by: John Linhart <jan@linhart.email>
Update test assertions to expect 404 instead of 403 for non-existent projects in testSelectEntityTypeActionNotFound and testAddEntityActionNotFound methods.
|
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.
I see no issues in the code and I've tested the UI and found no issues 👍
Ruth agreed with 👍 that the wording can be updated with an additional PR.
What this PR adds
Adds comprehensive project entity management with detailed view and permission controls.
Key Features
Future Extensions
This foundation enables:
The architecture is designed to be extensible for future project management enhancements.
Counts column
Add/Remove managment