Skip to content

Conversation

@DA-344
Copy link
Contributor

@DA-344 DA-344 commented Mar 23, 2025

Summary

Adds support for receiving, setting, and updating a Scheduled Event's recurrence rule.

Documentation: resources/guild-scheduled-event

Needs testing.

Information

  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed).
  • This PR is not a code change (e.g. documentation, README, typehinting,
    examples, ...).

Checklist

  • I have searched the open pull requests for duplicates.
  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why.
  • I have updated the changelog to include these changes.

Comment on lines +321 to +322
If this recurrence rule was obtained from the API you will need to
:meth:`.copy` it in order to edit it.
Copy link
Member

Choose a reason for hiding this comment

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

one more question... why?

Copy link
Member

Choose a reason for hiding this comment

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

this

Copy link
Member

Choose a reason for hiding this comment

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

This class seems to serve more as a dataclass-ish than an API object. Which is fine, but having an edit method here, and then disallowing editing attributes seems weird. This class should realistically not even hold self._state (I believe). It would probably make everything easier.

@Lulalaby
Copy link
Member

Lulalaby commented Aug 2, 2025

Merge conflicts
(complicated discord feature, requires intense testing)

@Paillat-dev Paillat-dev added API reflection Discord API isn't correctly reflected hold: testing This pull request requires further testing labels Aug 6, 2025
@Lulalaby Lulalaby requested a review from a team as a code owner August 30, 2025 20:33
@Lulalaby Lulalaby force-pushed the master branch 2 times, most recently from b55c125 to 82659b2 Compare August 30, 2025 21:10
@Lulalaby Lulalaby removed the on hold label Aug 30, 2025
@Lulalaby Lulalaby requested a review from a team as a code owner September 1, 2025 12:31
@Paillat-dev Paillat-dev self-assigned this Sep 1, 2025
@Paillat-dev Paillat-dev self-requested a review September 1, 2025 15:41
@Lulalaby
Copy link
Member

Lulalaby commented Sep 1, 2025

@DA-344 went ahead and fixed the merge conflicts for you. please double check

Comment on lines +321 to +322
If this recurrence rule was obtained from the API you will need to
:meth:`.copy` it in order to edit it.
Copy link
Member

Choose a reason for hiding this comment

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

this

image: Optional[:class:`bytes`]
The cover image of the scheduled event
The cover image of the scheduled event.
recurrence_rule: Optional[:class:`ScheduledEventRecurrenceRule`]
Copy link
Member

Choose a reason for hiding this comment

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

If this is provided, is start_date ignored ?

Copy link
Member

@Paillat-dev Paillat-dev left a comment

Choose a reason for hiding this comment

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

Also see merge conflicts

self.exceptions: list[Object] = list(
map(
Object,
data.get("guild_scheduled_events_exceptions", []) or [],
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
data.get("guild_scheduled_events_exceptions", []) or [],
data.get("guild_scheduled_events_exceptions") or [],

return f"<ScheduledEventRecurrenceRule start_date={self.start_date} frequency={self.frequency} interval={self.interval}>"

@property
def weekdays(self) -> list[ScheduledEventWeekday]:
Copy link
Member

Choose a reason for hiding this comment

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

I've been thinking about this again. If you can, I think we should just make these normal attributes. Realistically, it's more likely to confuse someone than prevent misuse. See my comment on edit below as well

Comment on lines +321 to +322
If this recurrence rule was obtained from the API you will need to
:meth:`.copy` it in order to edit it.
Copy link
Member

Choose a reason for hiding this comment

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

This class seems to serve more as a dataclass-ish than an API object. Which is fine, but having an edit method here, and then disallowing editing attributes seems weird. This class should realistically not even hold self._state (I believe). It would probably make everything easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API reflection Discord API isn't correctly reflected hold: testing This pull request requires further testing priority: medium Medium Priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants