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

Try to fix model request subscribeTo: missing unusedSubscriptions #4245

Merged
merged 5 commits into from
Oct 18, 2024

Conversation

reiterl
Copy link
Member

@reiterl reiterl commented Oct 10, 2024

Resolve #4240

The subscription.unusedSubscriptions here can be undefined, now the code can handle it.

@reiterl reiterl added the bug label Oct 10, 2024
@reiterl reiterl added this to the 4.2 milestone Oct 10, 2024
Copy link
Member

@bastianjoel bastianjoel left a comment

Choose a reason for hiding this comment

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

The error happens because there is a meeting specific subscription that is missing a unused subscription. Please fix this instead of just suppressing the error message.

Also please log a warning to the console if subscription.unusedSubscription is undefined at that place.

@bastianjoel bastianjoel assigned reiterl and unassigned bastianjoel and Elblinator Oct 11, 2024
@bastianjoel
Copy link
Member

bastianjoel commented Oct 11, 2024

Also PLEASE improve you PR naming. The title does not provide any helpful information. Something like "Fix model request subscribeTo handle undefined unusedSubscription" would be better.

@reiterl reiterl changed the title Small fix to subscribeTo Try to fix model request subscribeTo: missing unusedSubscriptions Oct 11, 2024
@reiterl
Copy link
Member Author

reiterl commented Oct 11, 2024

@bastianjoel What is this unusedSubscriptions for? In the interface the field is optional.

@reiterl
Copy link
Member Author

reiterl commented Oct 11, 2024

Added a warning to the modelRequestService. The problematic subscription is los_first_contribution. Add unusedWhen in some los components.

@bastianjoel bastianjoel assigned reiterl and unassigned bastianjoel Oct 18, 2024
@Elblinator Elblinator enabled auto-merge (squash) October 18, 2024 09:16
@reiterl reiterl removed their assignment Oct 18, 2024
@Elblinator Elblinator merged commit b9ae41b into OpenSlides:main Oct 18, 2024
2 checks passed
@reiterl reiterl deleted the 4240-fix-for-subscribeTo branch October 18, 2024 11:29
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.

Setting 'first speech' throws errors in speaking lists
3 participants