[WIP] 5104: Require event organizer #68
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #68 +/- ##
============================================
- Coverage 0.57% 0.57% -0.01%
Complexity 1205 1205
============================================
Files 156 156
Lines 4361 4362 +1
============================================
Hits 25 25
- Misses 4336 4337 +1
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.
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)