Fix bug with /audit unable to retrieve users #527
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(@Taz03 for visibility) |
marko-radosavljevic
previously approved these changes
Aug 25, 2022
|
I like the new order of methods and refactorings. It did make the review a lot harder tho haha ❤️ |
Zabuzard
force-pushed
the
bugfix/audit_retrieve_users
branch
from
a71c5a9 to
b8ced96
Compare
|
rebase, no code change |
|
Kudos, SonarCloud Quality Gate passed! |
marko-radosavljevic
approved these changes
Aug 27, 2022
|
Merging this, seems to be fine - if not, we can act afterwards. Want to accelerate development for this, need to get out a release for all the major improvements. Also, its weekened and I want to code a bit 😄 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
The latest rework of
/auditmessed up some stuff and had a few bugs. This fixes it, closes #522 .Details
The most prominent issue was incorrect usage of JDAs actions:
jda.retrieveUserById(id).complete()which blocked the JDA thread andjda.getUserById(id)which looked up (outdated) cache, failing to find the user.The issue has been fixed by properly using
RestActions and theirmap/flatMapchains. Obviously this uglifies the code a bit - but many new helper methods to the rescue.Apart from that, there have been some minor improvements on the UX:
As well as a few code readability improvements here and there, and some minor safety bugfixes like
page <= 1,page >= totalPagesinstead of==checks.Last, the methods have been re-ordered in time they are used to increase the readability flow - unfortunately that makes the review a bit nasty due to bad GitHub diffing.
Tests
Improvements are well tested, all edge cases included:
User totally unknown to the guild (= user with no actions):
User with one page actions
User with many pages actions (max page size was reduced to 3 for the test)
