Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #68 +/- ##
============================================
+ Coverage 0.57% 3.71% +3.14%
- Complexity 1205 1248 +43
============================================
Files 156 162 +6
Lines 4361 4438 +77
============================================
+ Hits 25 165 +140
+ Misses 4336 4273 -63
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rimi-itk
force-pushed
the
feature/5110-require-event-organizer
branch
2 times, most recently
from
4f8f753 to
619a553
Compare
rimi-itk
force-pushed
the
feature/5110-require-event-organizer
branch
from
619a553 to
92edb10
Compare
rimi-itk
changed the title
| ->where(':user MEMBER OF o.users') | ||
| ->setParameter('user', $this->getUser()) | ||
| ); | ||
| $organization = AssociationField::new('organization') |
There was a problem hiding this comment.
A few notes:
- I think the original if/else construct has better readability but that is highly subjective
$organizationFieldwould better signal what is assigned to the vaiable
As for the actual business logic, the intent was:
organizeris ALWAYS requirededitorandadminusers can set organizer from all available organizers.organization-editor/admincan only select form organizations they belong to (they should ALWAYS belong to at least one organization, but I don't think this is enforced)
One further possible improvement (as stated in the linked ticket) would be for organization-editor/admin users that only belong to one organization. If we could set that organization as default value, and disable the field.
| $organization | ||
| ->setFormTypeOption('choices', $userOrganizations) | ||
| // Make sure that the user is not forced to make a choice if none exists. | ||
| ->setRequired($userOrganizations->count() > 0); |
There was a problem hiding this comment.
Choice should exist, so better to throw an exception.
# Conflicts: # CHANGELOG.md # README.md # config/packages/security.yaml
…nizations Add EasyAdmin filters to UserCrudController for organizations, has/no organization, roles (JSON-aware), and email. Add console command app:user:list-without-organization to list non-editor users with no organization. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of displaying a summary of all validation errors, scroll to and focus the first invalid field. Remove the now-unused form-errors-summary container and its translation keys. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
turegjorup
changed the title
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rimi-itk
left a comment
rimi-itk
left a comment
There was a problem hiding this comment.
This looks good! I've added some comments and suggestions.
| if (firstInvalid) { | ||
| firstInvalid.scrollIntoView({ behavior: 'smooth', block: 'center' }); | ||
| firstInvalid.focus({ preventScroll: true }); | ||
| } |
There was a problem hiding this comment.
This is a really good idea! 🎉
| $editorOrAbove = [ | ||
| UserRoles::ROLE_EDITOR->value, | ||
| UserRoles::ROLE_ADMIN->value, | ||
| UserRoles::ROLE_SUPER_ADMIN->value, | ||
| ]; |
There was a problem hiding this comment.
We should be able to compute this based on the role hierarchy, but we'll leave that for later.
| ->hideOnIndex(); | ||
| } | ||
|
|
||
| yield TextField::new('created_by') |
There was a problem hiding this comment.
We should use the entity property names (rather than database column names) to defined fields , i.e. TextField::new('createdBy') etc. (cf. https://symfony.com/bundles/EasyAdminBundle/current/fields.html).
There was a problem hiding this comment.
Can you “extend from the original templates and change only the parts you want to override” in the “new” and “edit” templates?
Co-authored-by: Mikkel Ricky <rimi@aarhus.dk>
Co-authored-by: Mikkel Ricky <rimi@aarhus.dk>
Co-authored-by: Mikkel Ricky <rimi@aarhus.dk>
3 participants
Link to ticket
https://leantime.itkdev.dk/_#/tickets/showTicket/5104
Description
Screenshot of the result
If your change affects the user interface you should include a screenshot of the result with the pull request.
Checklist
If your code does not pass all the requirements on the checklist you have to add a comment explaining why this change
should be exempt from the list.
Additional comments or questions
EventCrudController.php, but I assume that it has something to do with that role be some kind of super user (that has access to everything)User::getOrganizations()andSELECT o FROM Organization o WHERE user MEMBER OF o.users(cf. changes inEventCrudController.php)