Skip to content

Make change reco line changes updatable#3170

Merged
reiterl merged 4 commits intoOpenSlides:mainfrom
luisa-beerboom:3169-make-change-reco-line-spans-updatable
Nov 7, 2025
Merged

Make change reco line changes updatable#3170
reiterl merged 4 commits intoOpenSlides:mainfrom
luisa-beerboom:3169-make-change-reco-line-spans-updatable

Conversation

@luisa-beerboom
Copy link
Copy Markdown
Member

Closes #3169

Please test this with real data as well. Code may break if data corrupt.

Comment on lines +90 to +95
sorted_reco_data: list[tuple[int, int, int]] = sorted(
[
(reco["line_from"], reco["line_to"], id_)
for id_, reco in motion_reco_data.items()
]
)
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.

Can be directly sorted.

Suggested change
sorted_reco_data: list[tuple[int, int, int]] = sorted(
[
(reco["line_from"], reco["line_to"], id_)
for id_, reco in motion_reco_data.items()
]
)
sorted_reco_data: list[tuple[int, int, int]] = sorted(
(reco["line_from"], reco["line_to"], id_)
for id_, reco in motion_reco_data.items()
)

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.

Done

)

def test_update_with_line_changes_multi_motion(self) -> None:
self.create_motions_with_line_changes()
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.

This is basically the only test using two motions. So I would suggest passing such a list and use [1] as default.

Suggested change
self.create_motions_with_line_changes()
self.create_motions_with_line_changes([1, 2])

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.

Done differently

@reiterl
Copy link
Copy Markdown
Member

reiterl commented Oct 22, 2025

The code should check, that:

  • line_from <= line_to
  • 1 <= line_from and line_to <= max line of motion (I don't think this is done so far)
  • [line_from, line_to] doesn't intersect with other recos.

@luisa-beerboom
Copy link
Copy Markdown
Member Author

The code should check, that:

* line_from <= line_to

Already doing that

* 1 <= line_from and line_to <= max line of motion (I don't think this is done so far)

To quote our wiki : The complete diff and line-number algorithms are done in the client.
The backend literally has no idea what the maximum line number is, and I'm not reimplementing that logic here. This needs to be ensured by the client instead.

* [line_from, line_to] doesn't intersect with other recos.

Already doing that

Copy link
Copy Markdown
Member

@hjanott hjanott left a comment

Choose a reason for hiding this comment

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

For what I requested; I like.

@hjanott hjanott assigned luisa-beerboom and unassigned hjanott Nov 4, 2025
@luisa-beerboom luisa-beerboom removed their assignment Nov 4, 2025
@reiterl
Copy link
Copy Markdown
Member

reiterl commented Nov 5, 2025

The max line number check is implemented in the client. We don't want to reimplement the line number logic here.

@reiterl reiterl merged commit 102ce9f into OpenSlides:main Nov 7, 2025
6 checks passed
vkrasnovyd pushed a commit to vkrasnovyd/openslides-backend that referenced this pull request Nov 25, 2025
* Make change reco line changes updatable
* Requested changes
vkrasnovyd pushed a commit to vkrasnovyd/openslides-backend that referenced this pull request Dec 1, 2025
* Make change reco line changes updatable
* Requested changes

Co-authored-by: Viktoriia Krasnovyd viktoriia.krasnovyd@intevation.de
vkrasnovyd pushed a commit to vkrasnovyd/openslides-backend that referenced this pull request Dec 1, 2025
* Make change reco line changes updatable
* Requested changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Röntgen projectname Zeeman projectname

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Backend for change reco length editing

4 participants