Skip to content

Conversation

ankitsmt211
Copy link
Member

@ankitsmt211 ankitsmt211 commented Nov 2, 2023

  • default avatar if user do not have one
  • remove embed from warning message to user
  • minor fixes

if avatar is missing:
image

if present:
image

User warnings:
Before:
image

Now:
image

* default avatar if user do not have one
* remove embed from warning message to user
* minor fixes
@ankitsmt211 ankitsmt211 requested review from a team as code owners November 2, 2023 12:35
@Zabuzard Zabuzard added bug Something isn't working priority: major labels Nov 2, 2023
@ankitsmt211
Copy link
Member Author

ankitsmt211 commented Nov 2, 2023

image
There's one more instance like this, where author is nullable so i can't use effectiveUrl. Any suggestions around this? or should i leave em as it is?

@Zabuzard
Copy link
Member

Zabuzard commented Nov 3, 2023

According to the JDA doc, the avatar url param is Nullable:

doc

Why again did our previous code crash? null should be supported here by JDA.

@Zabuzard
Copy link
Member

Zabuzard commented Nov 3, 2023

The bug was not giving null to setAuthor(...), it was about us invoking a method on sth that was null, see:

code

@ankitsmt211
Copy link
Member Author

The bug was not giving null to setAuthor(...), it was about us invoking a method on sth that was null, see:

code

you are right about that, i'll discard other changes but i still think effectiveAvatar is better otherwise we get this
image

With effectiveAvatar, its either userAvatar or a default one.

@Zabuzard
Copy link
Member

Zabuzard commented Nov 6, 2023

sure thing

@ankitsmt211
Copy link
Member Author

ankitsmt211 commented Nov 6, 2023

Changes are not significant, i have replaced avatar url only in all files. Few more bug fixes in TransferFeature class. I have tested the changes in transfer-feature and report, mute, audit, ban. Like it should, displays a default avatar if none is present for user. I couldn't verify scam blocker and log routine, but i assume it works.

@ankitsmt211 ankitsmt211 requested a review from Zabuzard November 6, 2023 19:18
@ankitsmt211 ankitsmt211 requested a review from Taz03 November 11, 2023 04:30
Taz03
Taz03 previously approved these changes Nov 11, 2023
@ankitsmt211 ankitsmt211 merged commit 873faef into Together-Java:develop Nov 13, 2023
@Zabuzard Zabuzard mentioned this pull request Nov 22, 2023
@ankitsmt211 ankitsmt211 deleted the transfer-embed-remove branch November 30, 2023 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: major
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants