Skip to content

Fix motion history entry with deleted meeting#3222

Merged
hjanott merged 7 commits intoOpenSlides:mainfrom
hjanott:fix_history_entry_meeting_id
Dec 4, 2025
Merged

Fix motion history entry with deleted meeting#3222
hjanott merged 7 commits intoOpenSlides:mainfrom
hjanott:fix_history_entry_meeting_id

Conversation

@hjanott
Copy link
Copy Markdown
Member

@hjanott hjanott commented Nov 28, 2025

When deleting a meeting that meeting will delete its motions which will then create new history entries after the relation handling. That needs to be accounted for when creating entries that are referring to be deleted meetings.

@hjanott hjanott added this to the 4.3 milestone Nov 28, 2025
@hjanott hjanott self-assigned this Nov 28, 2025
@hjanott hjanott added bug migration Introduces a new migration labels Nov 28, 2025
Comment on lines +32 to +43
deleted_meetings = self.reader.filter(
"meeting",
And(
FilterOperator("meta_deleted", "=", True),
Or(
*[
FilterOperator("id", "=", id_)
for id_ in potentially_affected_meetings
]
),
),
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You could also just do a get_many on the meeting ids.
Deleted meetings will be filtered out and you won't potentially have a filter with 100s of lines of conditions.
Like this I am afraid the datastore request may break with too large of an instance.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes. That was something I was afraid of, too. I returned the commented out code instead and altered it a little.

return None
events: list[BaseRequestEvent] = []
filter_ = And(
FilterOperator("original_model_id", "%=", "motion/%"),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it really only for motions?
Why not just do it for all collections?
Shouldn't affect the rest of the code, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The user history doesn't use the meeting_id and the assignment isn't affected as shown by the alteration of that meeting delete test. Doing it though for extra security as this has just little affect on the request.

Comment on lines +53 to +64
# meetings = self.reader.get_many(
# [
# GetManyRequestPart("meeting", list(affected_meetings), ["meta_deleted"])
# ]
# ).get("meeting", {})
# events = [
# *[
# RequestUpdateEvent(f"history_entry/{id_}", fields={"meeting_id": None})
# for id_, entry in potentially_broken_entries.items()
# if meetings[entry["meeting_id"]].get("meta_deleted") != True
# ],
# ]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Delete

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Using this now but taking advantage of the fact that no deleted meetings are returned.

@luisa-beerboom luisa-beerboom removed their assignment Dec 2, 2025
@hjanott hjanott enabled auto-merge (squash) December 4, 2025 15:54
@hjanott hjanott merged commit ce778d8 into OpenSlides:main Dec 4, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug migration Introduces a new migration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants