Add diff_version field
#3126
Add diff_version field
#3126
Conversation
vkrasnovyd
added
migration
luisa-beerboom
left a comment
luisa-beerboom
left a comment
There was a problem hiding this comment.
Generally looks good, i have some questions though.
| if instance.get("lead_motion_id") or datastore_instance.get("lead_motion_id"): | ||
| return [ | ||
| { | ||
| "type": (MotionErrorType.DIFF_VERSION), | ||
| "message": "You can define a diff_version only for the lead motion", | ||
| } | ||
| ] | ||
| return [] |
There was a problem hiding this comment.
Shouldn't there also be a requirement that normal motions need a diff version, or should it maybe be filled automatically?
There was a problem hiding this comment.
Valid point. @bastianjoel what is the desired backend behaviour in case no diff_version is provided for the normal motion?
There was a problem hiding this comment.
It can be left empty in that case. So no requirement to set that field needed.
There was a problem hiding this comment.
If you leave the possibility open to leave the field empty, what happens to lead motions that don't have the field set when the diff service is next updated?
Is there going to be a migration like in this PR every single time that happens?
Or are we just going to risk letting diffs for old motions break?
There was a problem hiding this comment.
If you leave the possibility open to leave the field empty, what happens to lead motions that don't have the field set when the diff service is next updated?
The compatibility layer will not be applied on these. I want to keep this option open for now.
Is there going to be a migration like in this PR every single time that happens?
No, if we decide that it needs to be required we can just set a default version in the client and apply the requirement check later.
3 participants
Needed for OpenSlides/openslides-client#5355
OpenSlides/openslides-meta#307