Skip to content

Auditlog cleanup and update #1586

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.

Sign up for GitHub

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

Jump to bottom
Merged
akiraveliara merged 44 commits into from
Aug 28, 2023
Merged

Auditlog cleanup and update #1586

akiraveliara merged 44 commits into from
Aug 28, 2023

Conversation

@Plerx2493
Copy link
Member

@Plerx2493 Plerx2493 commented Jul 7, 2023 •
edited
Loading

Summary

I started to cleanup the GetAuditLogs method on DiscordGuild and updated the AuditLogActionType Enum

Notes

Feel free to give me feedback and point out additional methods i can cleanup in this scope. I thought about rewriting other auditlog related sections but i dont know entirely how it works

Tasks

  • Implement missing entry types
  • Implement parsing of missing types
  • Implement missing AuditLog event

@Plerx2493
Copy link
Member Author

Plerx2493 commented Jul 7, 2023

i have not implemented a new type for the new actionTypes because i didnt know if we want to continue with this monster method

@Plerx2493
Copy link
Member Author

Plerx2493 commented Jul 7, 2023

The editorconfig is against using var. Should i replace it in all methods i clean up? its used everywhere

@akiraveliara
Copy link
Member

akiraveliara commented Jul 7, 2023

please do, and if you can, also run other format rules like file-scoped namespaces

@Plerx2493 Plerx2493 mentioned this pull request Jul 7, 2023
Closed
@Plerx2493
Copy link
Member Author

Plerx2493 commented Jul 7, 2023

@akiraveliara should i enforce all editorconfig warnings or only those which only affects files i already have changes in?

@Plerx2493
Copy link
Member Author

Plerx2493 commented Jul 7, 2023

There are several naming rules broken but they also make changes in DiscordClient.cs

@akiraveliara
Copy link
Member

akiraveliara commented Jul 7, 2023

only in files you touch for now, so as to avoid huge merge conflicts

@Plerx2493
Copy link
Member Author

Plerx2493 commented Jul 7, 2023

ok i got it

@Plerx2493
Copy link
Member Author

Plerx2493 commented Jul 7, 2023

image
145...

@Plerx2493
Copy link
Member Author

Plerx2493 commented Jul 7, 2023

Now its a huge PR for a minor change ^^

@akiraveliara
Copy link
Member

akiraveliara commented Jul 7, 2023

you can always make the PR extend to more audit log functionality if you want (:

@akiraveliara
Copy link
Member

akiraveliara commented Jul 7, 2023

but seriously, that's fine. it'll be a little annoying when it comes to reverting commits, but eh

@Plerx2493
Copy link
Member Author

Plerx2493 commented Jul 7, 2023

My plan is to implement the missing auditlogtypes. I will see what I can do after that

inftord
inftord reviewed Aug 11, 2023
View reviewed changes
Copy link
Member

@inftord inftord left a comment

Choose a reason for hiding this comment

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

more documentation required, but other than that LGTM

@Plerx2493
Copy link
Member Author

Plerx2493 commented Aug 11, 2023

may close #1569

I haven't had a chance to check it yet, but I can look at it later.

@Plerx2493
Copy link
Member Author

Plerx2493 commented Aug 11, 2023

more documentation required, but other than that LGTM

xml docs on the new methods or anything else? I have those in mind

@inftord
Copy link
Member

inftord commented Aug 11, 2023

yes, xmldocs

@Plerx2493 Plerx2493 requested a review from inftord August 12, 2023 02:01
@Plerx2493 Plerx2493 mentioned this pull request Aug 12, 2023
Closed
akiraveliara
akiraveliara approved these changes Aug 12, 2023
View reviewed changes
Copy link
Member

@akiraveliara akiraveliara left a comment •
edited
Loading

Choose a reason for hiding this comment

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

lgtm, will wait for preliminary test results before merging - if you want to split the types off into their own files, feel free

@Plerx2493
Copy link
Member Author

Plerx2493 commented Aug 12, 2023

lgtm, will wait for preliminary test results before merging - if you want to split the types off into their own files, feel free

currently working on it

@Plerx2493
Copy link
Member Author

Plerx2493 commented Aug 12, 2023

Should be ready now

inftord
inftord approved these changes Aug 12, 2023
View reviewed changes
Copy link
Member

@inftord inftord left a comment

Choose a reason for hiding this comment

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

LGTM

@akiraveliara akiraveliara mentioned this pull request Aug 13, 2023
Open
36 tasks
@Plerx2493
Copy link
Member Author

Plerx2493 commented Aug 14, 2023 •
edited
Loading

Missing change keys:

DiscordAuditLogAutoModerationRuleEntry:

  • $add_keyword_filter
  • $remove_keyword_filter

DiscordAuditLogIntegrationEntry:

  • type
  • name

DiscordAuditLogThreadEventEntry:

  • flags

@akiraveliara
Copy link
Member

akiraveliara commented Aug 28, 2023

once the change keys are added we can merge this as far as i'm concerned

@Plerx2493
Copy link
Member Author

Plerx2493 commented Aug 28, 2023

should be ready to merge

@Plerx2493 Plerx2493 requested a review from akiraveliara August 28, 2023 15:43
@akiraveliara akiraveliara merged commit 5bc3201 into DSharpPlus:master Aug 28, 2023
@Plerx2493 Plerx2493 mentioned this pull request Aug 29, 2023
Merged
@Plerx2493 Plerx2493 deleted the dev/auditlogUpdate branch October 5, 2023 13:48
OoLunar pushed a commit that referenced this pull request Oct 3, 2024
@Plerx2493 @kubikpatrick
@akiraveliara @VelvetToroyashi
Auditlog cleanup and update (#1586)
676ea66 
* Update AuditLogActionType

* Cleanup var naming, minor refactors (ToArra to avoid multiple enumerations)

* remove misclick

* Update AuditLogActionType

* Apply editorconfig

* Apply editorconfig

* Fix Typos and use new()

* nuke old auditLog abstraction, add new Auditlogentry types and start full rewrite of Guild#GetAuditLogsAsync

* Make PropertyChange a struct and start to split the auditlogentry parsing into AuditLogParser

* fix indentation

* Continue moving

* Continue moving

* Continue moving

* The great moving

* Add new Event and rename the GuildBan intent to GuildModeration (according to the discord docs)

* init Event...

* Fix caches for ResponsibleUser

* Use the guild cache

* Update data representation

* Update data representation

* Fix Permission updates

* Add check to minimize api calls

* Re-added a flag that I had deleted + headers formatting. (#1587)

* Added missing flag, and remove useless flag

* Added headers to Formatter.cs

* Re-added a flag that I had deleted.

* System alert channel add (#1588)

* SafetyAlertsChannel property added.

* Handle nullable safety alerts channel ID. (#1597)

* fix: Use `DiscordJson` to deserialize GDMs

* fix indentation

* why are you running

* Add missing parser methods

* Finish last parsing Method

* Add missing docs and formatting

* Update and use thread cache

* Rename autogenerated parameter

* Update PropertyChange

* Adding xmldocs, revert Intent renaming, update method return types

* ups

* Add toggleable logging

* Great moving the 2nd

* Fix missing init

* Fix wrong casting and rename member

* add missing key on threads and integration

* add missing key on automod entries

---------

Co-authored-by: P A T R I C K <85168257+Patrickooos@users.noreply.github.com>
Co-authored-by: Livia Silvercrown <61018569+akiraveliara@users.noreply.github.com>
Co-authored-by: Velvet Toroyashi <42438262+VelvetToroyashi@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@inftord inftord inftord approved these changes

@akiraveliara akiraveliara Awaiting requested review from akiraveliara

Assignees

No one assigned

Labels

big-change core enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Plerx2493 @akiraveliara @inftord @kubikpatrick @VelvetToroyashi