Skip to content
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

Fix motion create form #4286

Merged
merged 3 commits into from
Oct 23, 2024
Merged

Conversation

Elblinator
Copy link
Member

closes #4285

@Elblinator Elblinator added the bug label Oct 22, 2024
@Elblinator Elblinator added this to the 4.2 milestone Oct 22, 2024
@Elblinator Elblinator marked this pull request as ready for review October 22, 2024 15:07
Comment on lines 215 to 217
if (update[key] === null || update[key].length === 0) {
delete update[key];
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not a good idea: null's the value we use to signify that a value is to be deleted, removing all keys with that value is going to remove that functionality. Also you can't guarantee that every key is an array.

What I think you really want to do, is removing certain keys depending on whether the user is even allowed to set them according to his permissions

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the motion.create it is enough to remove every null/empty array from the payload. The permissions do not have to be verified, given that a user can only send null/[] for the fields they are not allowed to change

For the motion.update it is wrong to remove empty arrays or null values

Copy link

@MSoeb MSoeb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a functional perspective it works.

@luisa-beerboom luisa-beerboom merged commit e6c6e70 into OpenSlides:main Oct 23, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delegate unable to create a new motion
4 participants