Formatting issue when in subscription update

[narthollis]

When I received a subscription update, I noticed that the there were unnecessary spaces in the name of the user I had newly subecribed to.

I suspect this has to do with the code being written to handle a list of subscriptions but only getting one.

[TheDarkLordSano]

I agree complete. Trying to figure out a solution but it's low priority. imho

On Mon, Jul 31, 2017 at 5:18 AM, Nicholas Steicke notifications@github.com wrote:

When I received a subscription update, I noticed that the there were unnecessary spaces in the name of the user I had newly subecribed to.

[image: image] https://user-images.githubusercontent.com/67732/28777327-cb8c30c0-7639-11e7-9824-1917913c8a04.png

I suspect this has to do with the code being written to handle a list of subscriptions but only getting one.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/TheDarkLordSano/HFYBeetusBot/issues/20, or mute the thread https://github.com/notifications/unsubscribe-auth/ACW7cdwlt6ugF9qnirOv9G924MWIzY-Nks5sTcYvgaJpZM4OoTGl .

[narthollis]

Yeah, fair enough. Just thought I would get the bug down in case you haven't seen it yet

On 31 Jul. 2017 23:47, "TheDarkLordSano" notifications@github.com wrote:

I agree complete. Trying to figure out a solution but it's low priority. imho

On Mon, Jul 31, 2017 at 5:18 AM, Nicholas Steicke notifications@github.com wrote:

When I received a subscription update, I noticed that the there were unnecessary spaces in the name of the user I had newly subecribed to.

[image: image] https://user-images.githubusercontent.com/67732/ 28777327-cb8c30c0-7639-11e7-9824-1917913c8a04.png

I suspect this has to do with the code being written to handle a list of subscriptions but only getting one.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/TheDarkLordSano/HFYBeetusBot/issues/20, or mute the thread https://github.com/notifications/unsubscribe-auth/ ACW7cdwlt6ugF9qnirOv9G924MWIzY-Nks5sTcYvgaJpZM4OoTGl .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/TheDarkLordSano/HFYBeetusBot/issues/20#issuecomment-319080129, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEIlJNZgGkIYt_YgopRHJD0Tn4l5hAZks5sTeHfgaJpZM4OoTGl .

[TheDarkLordSano]

Have at it, I'll be uploading a new batch of code tonight.

On Mon, Jul 31, 2017 at 7:24 AM, Nicholas Steicke notifications@github.com wrote:

Yeah, fair enough. Just thought I would get the bug down in case you haven't seen it yet

On 31 Jul. 2017 23:47, "TheDarkLordSano" notifications@github.com wrote:

I agree complete. Trying to figure out a solution but it's low priority. imho

On Mon, Jul 31, 2017 at 5:18 AM, Nicholas Steicke < notifications@github.com> wrote:

When I received a subscription update, I noticed that the there were unnecessary spaces in the name of the user I had newly subecribed to.

[image: image] https://user-images.githubusercontent.com/67732/ 28777327-cb8c30c0-7639-11e7-9824-1917913c8a04.png

I suspect this has to do with the code being written to handle a list of subscriptions but only getting one.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/TheDarkLordSano/HFYBeetusBot/issues/20, or mute the thread https://github.com/notifications/unsubscribe-auth/ ACW7cdwlt6ugF9qnirOv9G924MWIzY-Nks5sTcYvgaJpZM4OoTGl .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/TheDarkLordSano/HFYBeetusBot/issues/20#issuecomment- 319080129, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEIlJNZgGkIYt_ YgopRHJD0Tn4l5hAZks5sTeHfgaJpZM4OoTGl .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/TheDarkLordSano/HFYBeetusBot/issues/20#issuecomment-319082288, or mute the thread https://github.com/notifications/unsubscribe-auth/ACW7cf3wsbxLGsDeHNgCb_pYkG-E53gMks5sTeOLgaJpZM4OoTGl .

[bontrose]

well shoot, i was going to report this. guess i don't sub often enough.

[Glitchfinder]

Well, I figured out why the text renders like that, but not how it happens. It's an odd case, but if you hop over to line 79 of inbox.py (and line 86 of the same), you'll see a .join command. When performed on a list, it will join all items together with spaces in between. Based on the code, that variable should always, without exceptions, be a list. However, the output on a single user suggests otherwise, because when a join is performed on a string, it places the spaces between each and every letter in the string. I ran some example code earlier to test the known output and a potential workaround, which is below:

import re
user_regex = re.compile('\/([A-Za-z0-9_-]{1,})')
users = user_regex.findall("Subscribe: /Glitchkey")

print(users)
print(' '.join("Glitchkey"))
print(' '.join(users))

test = "Glitchkey"

if isinstance(test, basestring):
    print(test)
else:
    print(' '.join(test))

test = users

if isinstance(test, basestring):
    print(test)
else:
    print(' '.join(test))

The output for that test, in order, is:

• What the raw users variable should be
• What the end result is for a generic string, demonstrating the issue reported.
• What the end result is for a single string in a list, which the users variable is /intended/ to be.
• A test case where the input is assumed to be either a single string or a list of any number of strings, fed a single string.
• The same test case as above, fed a list of one string.

You can actually test the code yourself with a site like Python Tutor.

[Glitchfinder]

While testing the recent subscription issues, I've confirmed a bug related to this issue: The list of users does not function as intended. The message only confirms and shows the last user in the list, if you subscribe to multiple users at once.

[Glitchfinder]

All right, while I'm still not sure exactly what's causing the issue with the array, I know of a proper way to handle this issue, including making sure that the message only shows up when an operation is successful. To do so, I would change lines 30-71 of inbox.py to the following:

        if "unsubscribe" in message.body.lower() or "unsubscribe" in message.subject.lower():
            users = extract_users(message.body)
            userlist = list()
            success = false

            for user in users:
                logger.info("Removing subscription from %s to %s" % (user, message.author))
                result = Subscriptions.objects.filter(subscribed_to__iexact=user, subscriber=message.author).delete()
                if result[0] != 0
                    userlist.append("/u/" + user)
                    success = true

            if success
                send_message.delay(
                    str(message.author),
                    "Your Current Subscriptions",
                    construct_pm(
                        message.author,
                        "unsubscribed from",
                        userlist,
                        config.get_subscriptions(message.author)
                    )
                )
            # If you want to create and add an error message for no subscriptions being removed, it should be an else statement in place of this comment

        elif "subscribe" in message.body.lower() or "subscribe" in message.subject.lower():
            users = extract_users(message.body)
            userlist = list()
            success = false

            for user in users:
                if user.lower() == 'u':
                    continue
                else:
                    logger.info("Added subscription from %s to %s" % (user, message.author))
                    try:
                        Subscriptions.objects.create(subscriber=message.author, subscribed_to=user)
                        userlist.append("/u/" + user)
            success = true
                    except IntegrityError as e:
                        if 'unique constraint' in e.args[0]:
                            logger.info("Subscription does not exist")
                            continue

            if success
                send_message.delay(
                    str(message.author),
                    "Your Current Subscriptions",
                    construct_pm(
                        message.author,
                        "subscribed to",
                        userlist,
                        list(Subscriptions.filter(subscriber=message.author).values_list("subscribed_to",flat=True))
                    )
                )
            # If you want to create and add an error message for no subscriptions being added, it should be an else statement in place of this comment

These changes will make sure that the full list of successful subscriptions/unsubscriptions is shown, as well as resolving the visual issue mentioned above. It also has allowances to create an error message for when no subscriptions/unsubuscriptions are successful, and I can easily add in code to track failed subscriptions/unsubscriptions in the event that you want to list those.

[TheDarkLordSano]

missing a few :, Capital T on true and Capital F on false. but looks good

going through testing right now

[Glitchfinder]

Oh, right. Never done much in Python aside from modifying a few export plugins I use. Worth noting that the code block above includes the lines I mentioned in #23, but doesn't include the suggestion I made there.