fix: GuildSchedule not being cached correctly + missing fields#3025
fix: GuildSchedule not being cached correctly + missing fields#3025Lumabots wants to merge 51 commits into
Conversation
|
Thanks for opening this pull request! This pull request can be checked-out with: git fetch origin pull/3025/head:pr-3025
git checkout pr-3025This pull request can be installed with: pip install git+https://github.com/Pycord-Development/pycord@refs/pull/3025/head |
|
Audit Logs still need to be rework (i didnt understand how it work yet) |
|
should we use use_cache_on_error, instead of only use cache ? that way we will be able to fetch first and if unavailable we will get |
Paillat-dev
left a comment
There was a problem hiding this comment.
If possible, make this pr not interfere with the recurrence one from anonymous. There will probably be merge conflicts once that one is merged, but they shouldn't both implement recurrence in a different way.
Co-authored-by: Paillat <[email protected]> Signed-off-by: Lumouille <[email protected]>
when anonymous pr will be merged i'll edit it to make it work |
25347b5 to
cc6ebbb
Compare
45025bd to
01e9fa8
Compare
Paillat-dev
left a comment
There was a problem hiding this comment.
Also please use from typing_extensions import deprecated and not typing_extensions.deprecated throughout
| return ScheduledEvent( | ||
| data=data, guild=self.guild, creator=self.creator, state=self._state | ||
| ) | ||
| data = await self._state.http.edit_scheduled_event( |
There was a problem hiding this comment.
Is there a reason why the if payload != {} guard was removed ?
| self.with_member = with_member | ||
| self.before = before | ||
| self.after = after | ||
| self.use_cache = use_cache |
There was a problem hiding this comment.
The behavior of use_cache should be documented and maybe a warning be thrown if limit is not None / with_member is false and use_cache is true, since only member objects are returned when using use_cache
There was a problem hiding this comment.
how could you want me to warn ? with a logging.warning ? with a warn.warning ?
| _enum_transformer(enums.NotificationLevel), | ||
| ), | ||
| "entity_metadata": (None, _transform_entity_metadata), | ||
| "location": (None, _transform_entity_metadata), |
There was a problem hiding this comment.
Still confused, location isn't supposed to exist as a top level property on Discord's side no ? So why even do we want to set this if it's never there in the first place ? This code is a mess so I may be wrong here but I'm confused as to why we would handle something Discord doesn't send.
| @@ -279,7 +295,7 @@ class AuditLogChanges: | |||
| "status": (None, _enum_transformer(enums.ScheduledEventStatus)), | |||
| "entity_type": ( | |||
| "location_type", | |||
There was a problem hiding this comment.
This renames it to location_type which makes it go against the entire pr's idea. But at the same time, it would be brekaing to change it without backwards compat.
There was a problem hiding this comment.
i dont understand ur statement, but i didnt really change auditlogs as now its working fine, and im scared touching anything would break it as the code is a big mess imo
There was a problem hiding this comment.
@Pycord-Development/all-contributors Can someone take a look at this audit log stuff as well that is not me ? I'm worried I'm missing something and this is quite a mess
…-Development/pycord into on_scheduled_event_delete

Summary
summary is not complete, just check the code
Added
New Enums :
ScheduledEventRecurrenceFrequency (yearly, monthly, weekly, daily)
ScheduledEventRecurrenceWeekday (monday-sunday)
ScheduledEventRecurrenceMonth (january-december)
New Classes:
ScheduledEventEntityMetadata (location)
ScheduledEventRecurrenceNWeekday Represents n-weekday entries (e.g., "1st Tuesday")
ScheduledEventRecurrenceRule: Full recurrence rule with validation
Auto-validates on to_payload() serialization
New fields Guild.create_scheduled_event():
Enforces Discord's constraints:
Information
examples, ...).
Checklist
type: ignorecomments were used, a comment is also left explaining why.