Enterprise Social Messaging Environment (ESME)
  1. Enterprise Social Messaging Environment (ESME)
  2. ESME-282

When a public message is resent by one user, no other users are able to resend it

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: Release 1.1
    • Fix Version/s: 1.3
    • Component/s: Server
    • Labels:
      None
    • Environment:
      Can be recreated on Stax or locally

      Description

      Consider a situation where there are three users:

      User A
      User B (follows user A)
      User C (follows user B)

      The issue: User A creates a new public message. User B resends this message. User C will see the message, but cannot resend the message.

      Interestingly, this does not appear to happen for messages in pools.

      1. resend1.png
        14 kB
        Vladimir Ivanov
      2. resend_usera.png
        6 kB
        Vladimir Ivanov
      3. userc_resend.png
        6 kB
        Vladimir Ivanov
      4. resend_userd.png
        6 kB
        Vladimir Ivanov

        Activity

        Hide
        Dick Hirsch added a comment -

        Closing based on very tests

        Show
        Dick Hirsch added a comment - Closing based on very tests
        Hide
        Vladimir Ivanov added a comment -

        I've just tested it, resend action works as expected for 4th user (see resend_userd.png image attached).

        Show
        Vladimir Ivanov added a comment - I've just tested it, resend action works as expected for 4th user (see resend_userd.png image attached).
        Hide
        Vladimir Ivanov added a comment -

        Thanks for comments guys. I had clicked on resend link for User C as well but hadn't checked whether message was actually resent (to 4th user). I'll be able to verify it this evening.

        Show
        Vladimir Ivanov added a comment - Thanks for comments guys. I had clicked on resend link for User C as well but hadn't checked whether message was actually resent (to 4th user). I'll be able to verify it this evening.
        Hide
        Ethan Jewett added a comment -

        It's possible that it's no longer an issue. It's also possible that some behavior changed during the timeline rewrite that either fixed this issue or invalidated this way of showing the issue. The test would be to try this with 4 users and actually have User C resend the message in Vladimir's example. If it is properly resent, then we are in good shape

        Show
        Ethan Jewett added a comment - It's possible that it's no longer an issue. It's also possible that some behavior changed during the timeline rewrite that either fixed this issue or invalidated this way of showing the issue. The test would be to try this with 4 users and actually have User C resend the message in Vladimir's example. If it is properly resent, then we are in good shape
        Hide
        Dick Hirsch added a comment -

        I tried this as well without the users following one another and I didn't see it either. Maybe Ethan fixed it during his other timeline-related changes

        Show
        Dick Hirsch added a comment - I tried this as well without the users following one another and I didn't see it either. Maybe Ethan fixed it during his other timeline-related changes
        Hide
        Vladimir Ivanov added a comment -

        Could someone confirm that this issue still exists? I've tried to reproduce it locally (screenshots attached) but either I didn't understand steps to reproduce or it was fixed somehow.

        Show
        Vladimir Ivanov added a comment - Could someone confirm that this issue still exists? I've tried to reproduce it locally (screenshots attached) but either I didn't understand steps to reproduce or it was fixed somehow.
        Hide
        Ethan Jewett added a comment - - edited

        Dick, I think you are right!

        (Warning, pseudo code coming up...)

        I think we can change the signature of the Resend(Message) case class to be Resend(Message, User), then just change this block to

        case Resend(msgId, uId) =>
        for (msg <- Message.find(msgId)) {
        if(uId == userId) {
        if (!msg.pool.defined_?)
        PopStatsActor ! PopStatsActor.IncrStats(ResendStat, msgId)

        Mailbox.find(By(Mailbox.message, msg),
        By(Mailbox.user, userId)).foreach { m =>
        m.resent(true).save
        _mailbox = _mailbox.map

        { case (`msgId`, r, _) => (msgId, r, true) case x => x }


        listeners.foreach(_ ! Resend(msgId))
        }
        for (id <- followers)
        Distributor !
        Distributor.AddMessageToMailbox(id, msg, ResendReason(userId))
        }
        }
        }

        This would affect code in a couple of other places but that would be easy to fix. It would be good to have someone else look at it to verify that we're reading this correctly.

        Show
        Ethan Jewett added a comment - - edited Dick, I think you are right! (Warning, pseudo code coming up...) I think we can change the signature of the Resend(Message) case class to be Resend(Message, User), then just change this block to case Resend(msgId, uId) => for (msg <- Message.find(msgId)) { if(uId == userId) { if (!msg.pool.defined_?) PopStatsActor ! PopStatsActor.IncrStats(ResendStat, msgId) Mailbox.find(By(Mailbox.message, msg), By(Mailbox.user, userId)).foreach { m => m.resent(true).save _mailbox = _mailbox.map { case (`msgId`, r, _) => (msgId, r, true) case x => x } listeners.foreach(_ ! Resend(msgId)) } for (id <- followers) Distributor ! Distributor.AddMessageToMailbox(id, msg, ResendReason(userId)) } } } This would affect code in a couple of other places but that would be easy to fix. It would be good to have someone else look at it to verify that we're reading this correctly.
        Hide
        Dick Hirsch added a comment -

        Looks like the problem is here:

        I think it is setting the vale in mailboxes of all users rather than just the user that sent the message. It might be that we have to change the db to have resent require user and boolean.

        UserActor.scala

        case Resend(msgId) =>
        for (msg <- Message.find(msgId)) {
        if (!msg.pool.defined_?)
        PopStatsActor ! PopStatsActor.IncrStats(ResendStat, msgId)

        Mailbox.find(By(Mailbox.message, msg),
        By(Mailbox.user, userId)).foreach { m =>
        m.resent(true).save
        _mailbox = _mailbox.map

        { case (`msgId`, r, _) => (msgId, r, true) case x => x }

        listeners.foreach(_ ! Resend(msgId))
        }
        for (id <- followers)
        Distributor !
        Distributor.AddMessageToMailbox(id, msg, ResendReason(userId))
        }
        }

        Show
        Dick Hirsch added a comment - Looks like the problem is here: I think it is setting the vale in mailboxes of all users rather than just the user that sent the message. It might be that we have to change the db to have resent require user and boolean. UserActor.scala case Resend(msgId) => for (msg <- Message.find(msgId)) { if (!msg.pool.defined_?) PopStatsActor ! PopStatsActor.IncrStats(ResendStat, msgId) Mailbox.find(By(Mailbox.message, msg), By(Mailbox.user, userId)).foreach { m => m.resent(true).save _mailbox = _mailbox.map { case (`msgId`, r, _) => (msgId, r, true) case x => x } listeners.foreach(_ ! Resend(msgId)) } for (id <- followers) Distributor ! Distributor.AddMessageToMailbox(id, msg, ResendReason(userId)) } }

          People

          • Assignee:
            Unassigned
            Reporter:
            Ethan Jewett
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development