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

Reject all daily events in import #3432

Merged
merged 1 commit into from
Feb 18, 2025
Merged

Conversation

MizukiTemma
Copy link
Member

Short description

This PR changes the event import functionality so no event with daily recurrence can be imported.

Proposed changes

  • Check the frequency and raise error if it is daily

Side effects

  • None?

Resolved issues

Fixes: #3431


Pull Request Review Guidelines

@MizukiTemma MizukiTemma added prio: high Needs to be resolved ASAP. effort: low Should be doable in <4h blocks-release This issue blocks a release labels Feb 14, 2025
@MizukiTemma MizukiTemma force-pushed the reject_all_daily_event_import branch from 8c60a72 to 5327e5a Compare February 14, 2025 15:22
@MizukiTemma MizukiTemma changed the title Reject all daily eventy in import Reject all daily events in import Feb 14, 2025
Copy link
Contributor

@JoeyStk JoeyStk left a comment

Choose a reason for hiding this comment

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

Thank you very much! The code runs as expected and didn't cause problems. I only have one concern / one thing we should think of.
I imported the test calendar from Nextcloud that we used to test the imports. In there we have one event that repeats every day and one that repeats every two days. Both failed to be imported. I take it that this is expected. I just want to double check with @osmers and/or @hauf-toni if this is really what we want :)

@osmers
Copy link

osmers commented Feb 17, 2025

I can see the problem that this is not very transparent for municipalities bcs they cannot see the import option and which events failed). On the other hand, so far we have no municipality using the import function (I think)...
Is this an issue we can tackle later? As in, approve this now but open another issue on how to deal with these events to that they fit the new event design?

@MizukiTemma
Copy link
Member Author

@osmers
Thank you for reply. An issue is opened for the thema #3443

Copy link
Contributor

@charludo charludo left a comment

Choose a reason for hiding this comment

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

Thank you! 🎉

@MizukiTemma MizukiTemma force-pushed the reject_all_daily_event_import branch from 5327e5a to 8ef6157 Compare February 18, 2025 09:20
@MizukiTemma MizukiTemma merged commit a6e3a8f into develop Feb 18, 2025
5 checks passed
@MizukiTemma MizukiTemma deleted the reject_all_daily_event_import branch February 18, 2025 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocks-release This issue blocks a release effort: low Should be doable in <4h prio: high Needs to be resolved ASAP.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reject all events with daily recurrence during import from external calendar
4 participants