-
Notifications
You must be signed in to change notification settings - Fork 24
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
Add orga setting 'require_duplicate_from' #2524
Add orga setting 'require_duplicate_from' #2524
Conversation
@luisa-beerboom Please review. |
If enable_duplicate_from_mandatory is set, committee manager are not allowed to create meetings, they need to user clone instead.
Correct, the committee admin should always use |
Some requirements were not documented and now need some implementing in this PR:
|
If a cm is not allowed to duplicate a meeting ("meeting.clone") they cannot create a new meeting, because the |
If that is possible then yes, only allow a cm to clone template meetings if the settings is active. |
if 'require_duplicate_from' organization setting is set.
@luisa-beerboom I have added the two changes @Elblinator requested. (With docs and tests). |
docs/actions/meeting.update.md
Outdated
@@ -202,3 +202,4 @@ This action doesn't allow for a meeting to be set as a template and have `locked | |||
- Admins of the meeting can modify group D | |||
- Users with CML `can_manage` or users with a OML of `can_manage_organization` can modify group E | |||
- Only users with OML `superadmin` can modify group F | |||
- Users need OML `can_manage_organization` to `set_as_template` if organization setting `require_duplicate_from` is set. |
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.
To keep the format, set_as_template
should be it's own group
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.
Now use a Group G for 'set_as_template'.
organization = self.datastore.get( | ||
ONE_ORGANIZATION_FQID, ["require_duplicate_from", "template_meeting_ids"] | ||
) | ||
if organization.get("require_duplicate_from"): | ||
if not has_organization_management_level( | ||
self.datastore, | ||
self.user_id, | ||
OrganizationManagementLevel.CAN_MANAGE_ORGANIZATION, | ||
) and not instance["meeting_id"] in ( | ||
organization.get("template_meeting_ids") or [] | ||
): |
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.
Either combine the if-conditions using and (i.e. organization.get("require_duplicate_from") and not has_organization_management_level(self.datastore, self.user_id, OrganizationManagementLevel.CAN_MANAGE_ORGANIZATION) and not instance["meeting_id"] in (organization.get("template_meeting_ids") or [])
) or do something like the following
organization = self.datastore.get( | |
ONE_ORGANIZATION_FQID, ["require_duplicate_from", "template_meeting_ids"] | |
) | |
if organization.get("require_duplicate_from"): | |
if not has_organization_management_level( | |
self.datastore, | |
self.user_id, | |
OrganizationManagementLevel.CAN_MANAGE_ORGANIZATION, | |
) and not instance["meeting_id"] in ( | |
organization.get("template_meeting_ids") or [] | |
): | |
if not instance["meeting_id"] in ( | |
organization.get("template_meeting_ids") or [] | |
): | |
organization = self.datastore.get( | |
ONE_ORGANIZATION_FQID, ["require_duplicate_from", "template_meeting_ids"], lock_result=False | |
) | |
if organization.get("require_duplicate_from") and not has_organization_management_level( | |
self.datastore, | |
self.user_id, | |
OrganizationManagementLevel.CAN_MANAGE_ORGANIZATION, | |
): |
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.
Use 'and' to combine the if-conditionals.
organization.get("template_meeting_ids") or [] | ||
): | ||
raise ActionException( | ||
"Committee manager cannot clone a non-template meeting, if duplicate-from is required." |
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.
"Committee manager cannot clone a non-template meeting, if duplicate-from is required." | |
"Committee manager cannot clone a non-template meeting if duplicate-from is required." |
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.
Rm the comma
organization = self.datastore.get( | ||
ONE_ORGANIZATION_FQID, ["require_duplicate_from"] | ||
) | ||
if organization.get("require_duplicate_from"): | ||
if set_as_template is not None and not has_organization_management_level( | ||
self.datastore, | ||
self.user_id, | ||
OrganizationManagementLevel.CAN_MANAGE_ORGANIZATION, | ||
): | ||
raise ActionException( | ||
"A meeting cannot be set as a template by a committee manager, if duplicate from is required." | ||
) |
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.
see my comment in meeting/clone.py
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.
Use 'and' to combine the if-conditionals and rm the comma.
Also a update of the meeting.update docs, use group G
I have worked on the changes for the CR. I hope, this is now okay. @luisa-beerboom pls review. |
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.
the datastore calls should not lock fields
Don't lock the fields of the two datastore calls, which get the organization. |
Resolve #2507