Implemented phone verification (only get) and 50 MiB file limit for nitro accounts #445

Almighty-Alpaca · 2017-08-25T20:41:25Z

If anyone knows what the mobile is exactly used for let me know so that I can update the javadoc for it as it's currently a bit vague.

…evel and added support for sending 50 MiB files when having nitro

randomnetcat · 2017-08-26T01:29:10Z

While you're there, could you fix: File is to big! (MessageChannel:511)

MinnDevelopment · 2017-08-26T03:41:11Z

src/main/java/net/dv8tion/jda/core/handle/UserUpdateHandler.java

-                    new SelfUpdateEmailEvent(
-                            api, responseNumber,
-                            oldEmail));
+            api.getEventManager().handle(new SelfUpdateEmailEvent(api, responseNumber, oldEmail));


Why did you change these events here? They have a consistent setup...

^ I wanna reiterate this comment, please don't change these as we have a consistent style for these event handling calls.

MinnDevelopment · 2017-08-26T05:12:53Z

Does having a registered phone number outweigh the other verification level requirements?

Sanduhr32 · 2017-08-26T07:51:13Z

Yes Minn. However the current verification level of a guild outweights the lower verification levels. And yes i tested it :)

MinnDevelopment · 2017-08-26T08:55:21Z

If that is the case this swtich has to be changed. And when the phone number is not null it should not break but return true from what I can tell.

Almighty-Alpaca · 2017-08-26T16:53:27Z

src/main/java/net/dv8tion/jda/core/entities/impl/GuildImpl.java

@@ -661,19 +661,22 @@ public boolean checkVerification()
            return true;
        if(canSendVerification)
            return true;
+
+        if (api.getSelfUser().getPhoneNumber() != null)


@MinnDevelopment @Sanduhr32 is this what you mean?

edit: I hate these snippets...

DuncanCasteleyn · 2017-08-26T17:17:17Z

src/main/java/net/dv8tion/jda/core/entities/impl/GuildImpl.java

        switch (verificationLevel)
        {
            case VERY_HIGH:
-                if(api.getSelfUser().getPhoneNumber() != null)
-                    break;
+                return false; // we already checked for a verified phone number


Shouldn't this be a break; to be consistent?

MinnDevelopment

I'm unsure whether you were done with this but it has been laying low for a bit now. There are several TODO comments I would like you to look into before we can merge this.

MinnDevelopment · 2017-09-12T10:48:59Z

src/main/java/net/dv8tion/jda/core/entities/EntityBuilder.java

@@ -80,6 +80,9 @@ public SelfUser createSelfUser(JSONObject self)
                .setVerified(self.getBoolean("verified"))
                .setMfaEnabled(self.getBoolean("mfa_enabled"))
                .setEmail(!self.isNull("email") ? self.getString("email") : null)
+                .setMobile(self.has("mobile") ? self.getBoolean("mobile") : false) 
+                .setPremium(self.has("premium") ? self.getBoolean("premium") : false)
+                .setPhoneNumber(!self.isNull("phone") ? self.getString("phone") : null)


This could be !self.isNull(x) && self.getBoolean(x) for the boolean values.

MinnDevelopment · 2017-09-12T10:49:32Z

src/main/java/net/dv8tion/jda/core/entities/MessageChannel.java

@@ -507,8 +507,8 @@ protected void handleResponse(Response response, Request<Message> request)
        Checks.notNull(file, "file");
        Checks.check(file.exists() && file.canRead(),
            "Provided file is either null, doesn't exist or is not readable!");
-        Checks.check(file.length() <= Message.MAX_FILE_SIZE,// TODO: deal with Discord Nitro allowing 50MB files.
-            "File is to big! Max file-size is 8MB");
+        Checks.check(file.length() <= (getJDA().getAccountType() == AccountType.CLIENT && getJDA().getSelfUser().isPremium() ? Message.MAX_FILE_SIZE_NITRO : Message.MAX_FILE_SIZE),


This ternary is pretty ugly, can we move this outside the check call?

MinnDevelopment · 2017-09-12T10:49:41Z

src/main/java/net/dv8tion/jda/core/entities/MessageChannel.java

        Checks.notNull(fileName, "fileName");

-        Checks.check(data.length <= Message.MAX_FILE_SIZE,   //8MB
-                "Provided data is too large! Max file-size is 8MB (%d)", Message.MAX_FILE_SIZE);
+        Checks.check(data.length <= (getJDA().getAccountType() == AccountType.CLIENT && getJDA().getSelfUser().isPremium() ? Message.MAX_FILE_SIZE_NITRO : Message.MAX_FILE_SIZE),


Same as above

MinnDevelopment · 2017-09-12T10:51:48Z

src/main/java/net/dv8tion/jda/core/entities/SelfUser.java

+     *
+     * @return The phone of the currently logged in account or null if there's no number associated
+     */
+    String getPhoneNumber() throws AccountTypeException;


Why do you specifically throw an AccountTypeException for these methods? I would suggest to actually just return w/e we parsed and document that for bots the value will most likely be false or null. It might be possible that discord will make some of these available to bots in the future (unlikely) so we should just go with that, imo.

I chose this because we already have the same be behavior for getEmail(). I also highly doubt that any of these will be available for bots anytime in the future.

MinnDevelopment · 2017-09-12T10:52:14Z

src/main/java/net/dv8tion/jda/core/entities/impl/GuildImpl.java

        switch (verificationLevel)
        {
+            case VERY_HIGH:
+                return false; // we already checked for a verified phone number


This should be a break; for consistency

MinnDevelopment · 2017-09-12T10:53:32Z

src/main/java/net/dv8tion/jda/core/handle/UserUpdateHandler.java

-                    new SelfUpdateEmailEvent(
-                            api, responseNumber,
-                            oldEmail));
+            api.getEventManager().handle(new SelfUpdateEmailEvent(api, responseNumber, oldEmail));


^ I wanna reiterate this comment, please don't change these as we have a consistent style for these event handling calls.

MinnDevelopment · 2017-09-12T10:53:55Z

src/main/java/net/dv8tion/jda/core/handle/UserUpdateHandler.java

+            JDAImpl.LOG.debug("");
+            String oldPhoneNumber = self.getPhoneNumber();
+            self.setPhoneNumber(phoneNumber);
+//            api.getEventManager().handle(new SelfUpdatePhoneNumberEvent(api, responseNumber, oldPhoneNumber));


When do you intend to add these events?

DV8FromTheWorld · 2017-10-04T05:58:11Z

I'd like to see some motion on this PR.

Almighty-Alpaca · 2017-10-04T14:44:58Z

Right... I completely forgot this still exists.

DuncanCasteleyn

Why is it called premium and not just Nitro? I think people would search docs for Nitro rather than premium, i might be confusing for other people.

DuncanCasteleyn

I'd suggest renaming the constant to so it contains the same name as the Boolean which is currently named premium.

DuncanCasteleyn · 2017-10-08T18:17:28Z

src/main/java/net/dv8tion/jda/core/entities/Message.java

+    /**
+     * Useful constant for the max allowed file size for message attachments on nitro accounts. Value: {@value #MAX_FILE_SIZE_NITRO} Bytes (50 MiB)
+     */
+    int MAX_FILE_SIZE_NITRO = 50 << 20; // 50 MiB


This isn't really consistent you are calling your Boolean premium to check if user is a Nitro users but here you are calling you constant MAX_FILE_SIZE_NITRO instead of MAX_FILE_SIZE_PREMIUM. I'd say either rename the Boolean to nitro or rename this constant.

MinnDevelopment · 2017-10-13T05:07:02Z

src/main/java/net/dv8tion/jda/core/entities/SelfUser.java

+     *
+     * @return The Discord Nitro status of the currently logged in account.
+     */
+    boolean isPremium() throws AccountTypeException;


We should call this isNitro to avoid unneeded confusion

MinnDevelopment · 2017-10-21T06:55:10Z

@Almighty-Alpaca can you please update this PR

MinnDevelopment · 2017-10-26T04:38:46Z

src/main/java/net/dv8tion/jda/core/events/self/SelfUpdateMobileEvent.java

+
+import net.dv8tion.jda.core.JDA;
+
+public class SelfUpdateMobileEvent extends GenericSelfUpdateEvent


This event needs documentation on class-level and on its methods.

MinnDevelopment · 2017-10-26T04:38:51Z

src/main/java/net/dv8tion/jda/core/events/self/SelfUpdateNitroEvent.java

+
+import net.dv8tion.jda.core.JDA;
+
+public class SelfUpdateNitroEvent extends GenericSelfUpdateEvent


This event needs documentation on class-level and on its methods.

MinnDevelopment · 2017-10-26T04:38:53Z

src/main/java/net/dv8tion/jda/core/events/self/SelfUpdatePhoneNumberEvent.java

+
+import net.dv8tion.jda.core.JDA;
+
+public class SelfUpdatePhoneNumberEvent extends GenericSelfUpdateEvent


This event needs documentation on class-level and on its methods.

MinnDevelopment · 2017-10-27T18:00:05Z

src/main/java/net/dv8tion/jda/core/handle/UserUpdateHandler.java

+
+        if (api.getAccountType() == AccountType.CLIENT && !Objects.equals(phoneNumber, self.getPhoneNumber()))
+        {
+            JDAImpl.LOG.debug("");


Almighty-Alpaca added 2 commits August 25, 2017 22:44

Fixed message sending on client accounts for very high verification l…

3de70f4

…evel and added support for sending 50 MiB files when having nitro

Fixed AccountTypeException for MessageChannel#sendMessage()

c228a8d

Almighty-Alpaca force-pushed the fix/phone-verification branch from 99608ee to c228a8d Compare August 25, 2017 20:46

MinnDevelopment reviewed Aug 26, 2017

View reviewed changes

MinnDevelopment added bug client type: discord issue seems to be an issue on discord's end labels Aug 26, 2017

A verified phone number now correctly outweights other requirements

7dfc8e7

Almighty-Alpaca commented Aug 26, 2017

View reviewed changes

DuncanCasteleyn suggested changes Aug 26, 2017

View reviewed changes

Merge branch 'master' into fix/phone-verification

7304622

MinnDevelopment modified the milestone: Upcoming Release Sep 6, 2017

MinnDevelopment requested changes Sep 12, 2017

View reviewed changes

Almighty-Alpaca added 2 commits October 7, 2017 18:34

Merge branch 'master' into fix/phone-verification

da3d46a

Fixes many things

d334009

DuncanCasteleyn reviewed Oct 8, 2017

View reviewed changes

DuncanCasteleyn suggested changes Oct 8, 2017

View reviewed changes

MinnDevelopment reviewed Oct 13, 2017

View reviewed changes

Almighty-Alpaca added 2 commits October 25, 2017 21:32

Merge branch 'master' into fix/phone-verification

64c2ce7

Renamed premium to nitro

017dae7

Almighty-Alpaca force-pushed the fix/phone-verification branch from c8a0e99 to 017dae7 Compare October 25, 2017 20:53

MinnDevelopment requested changes Oct 26, 2017

View reviewed changes

MinnDevelopment modified the milestones: Upcoming Release, Release 3.4 Oct 26, 2017

Almighty-Alpaca force-pushed the fix/phone-verification branch 2 times, most recently from 7026dad to a342114 Compare October 27, 2017 17:46

Added some docs

5580542

Almighty-Alpaca force-pushed the fix/phone-verification branch from a342114 to 5580542 Compare October 27, 2017 17:49

MinnDevelopment reviewed Oct 27, 2017

View reviewed changes

Small cleanup

341409e

MinnDevelopment approved these changes Oct 28, 2017

View reviewed changes

Almighty-Alpaca merged commit d6fc0bc into master Oct 29, 2017

MinnDevelopment deleted the fix/phone-verification branch October 29, 2017 18:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented phone verification (only get) and 50 MiB file limit for nitro accounts #445

Implemented phone verification (only get) and 50 MiB file limit for nitro accounts #445

Almighty-Alpaca commented Aug 25, 2017

randomnetcat commented Aug 26, 2017 •

edited

Loading

MinnDevelopment Aug 26, 2017

MinnDevelopment Sep 12, 2017

MinnDevelopment commented Aug 26, 2017

Sanduhr32 commented Aug 26, 2017

MinnDevelopment commented Aug 26, 2017

Almighty-Alpaca Aug 26, 2017 •

edited

Loading

DuncanCasteleyn Aug 26, 2017

MinnDevelopment left a comment

MinnDevelopment Sep 12, 2017

MinnDevelopment Sep 12, 2017

MinnDevelopment Sep 12, 2017

MinnDevelopment Sep 12, 2017

Almighty-Alpaca Oct 8, 2017

MinnDevelopment Sep 12, 2017

MinnDevelopment Sep 12, 2017

MinnDevelopment Sep 12, 2017

DV8FromTheWorld commented Oct 4, 2017

Almighty-Alpaca commented Oct 4, 2017

DuncanCasteleyn left a comment

DuncanCasteleyn left a comment •

edited

Loading

DuncanCasteleyn Oct 8, 2017 •

edited

Loading

MinnDevelopment Oct 13, 2017

MinnDevelopment commented Oct 21, 2017

MinnDevelopment Oct 26, 2017

MinnDevelopment Oct 26, 2017

MinnDevelopment Oct 26, 2017

MinnDevelopment Oct 27, 2017


		import net.dv8tion.jda.core.JDA;

		public class SelfUpdateMobileEvent extends GenericSelfUpdateEvent


		import net.dv8tion.jda.core.JDA;

		public class SelfUpdateNitroEvent extends GenericSelfUpdateEvent


		import net.dv8tion.jda.core.JDA;

		public class SelfUpdatePhoneNumberEvent extends GenericSelfUpdateEvent

Implemented phone verification (only get) and 50 MiB file limit for nitro accounts #445

Implemented phone verification (only get) and 50 MiB file limit for nitro accounts #445

Conversation

Almighty-Alpaca commented Aug 25, 2017

randomnetcat commented Aug 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MinnDevelopment commented Aug 26, 2017

Sanduhr32 commented Aug 26, 2017

MinnDevelopment commented Aug 26, 2017

Almighty-Alpaca Aug 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MinnDevelopment left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DV8FromTheWorld commented Oct 4, 2017

Almighty-Alpaca commented Oct 4, 2017

DuncanCasteleyn left a comment

Choose a reason for hiding this comment

DuncanCasteleyn left a comment • edited Loading

Choose a reason for hiding this comment

DuncanCasteleyn Oct 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MinnDevelopment commented Oct 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

randomnetcat commented Aug 26, 2017 •

edited

Loading

Almighty-Alpaca Aug 26, 2017 •

edited

Loading

DuncanCasteleyn left a comment •

edited

Loading

DuncanCasteleyn Oct 8, 2017 •

edited

Loading