When nominating another person, the first nominee’s message is re-posted

[pauldwaite]

Steps to reproduce

1. Visit the app
2. Click on the “Nominate with Facebook” button
3. Log into Facebook if necessary
4. Select a friend
5. Click on the submit button.
6. In another browser tab/window, visit your Facebook profile page, and delete the message you just posted via the app.
7. Back in the app, click on the “Nominate another Person” button
8. Select another friend, and post a message.
9. Visit your Facebook profile page again.

Expected behaviour

The second message you posted is present, and the first message is still deleted.

Actual behaviour

Both the first and the second message you posted are present.

Notes

I haven’t played too much with this to nail down what’s going on (I haven’t figured out Facebook API test users, and there are only so many test message I feel like I can post on my friends’ Facebook walls), but I’m assuming the first message gets re-posted when the message for the “another” friend goes up there.

Having mucked around with test users a bit, it seems that the second message gets posted for both the first and second person selected.

Ah yes — and we do have a console log for when we’re posting messages, and you can see both being logged when you do nominate again. Cool cool. (When I say cool cool, obviously it’s still a bug, but at least we know what’s happening. I’ll take a look.)

[pauldwaite]

In progress.

[pauldwaite]

So I presume the first Share view is still around when the second Share view’s submit button is pressed, and thus its shareForm submit event handler fires?

(I don’t really understand how Marionette views work, so I could be wrong.)

I tried calling destroy on the Share view in the success handler of the FB.api call in postToFacebook, but that stopped the success view showing up.

Maybe we’re meant to call empty on a region? (I’m not clear what a region is, or how to access the one that these views presumably show up in.)

[robertocarroll]

Going to talk this through from the beginning:

• app.js has a region defined by the CSS id "app".
• when the app starts the hello.js view is loaded into it
• there's another region inside that called facebookCallRegion which we show the button in when the API loads (Facebook.js)
• when the login is successful, it creates a new view (person.js)
• when is initialised, it starts a new collection and when the view is rendered it shows the collection in a childView (friends.js)
• each item in the list is listening for a selected event and if selected it triggers a new share view with the selected model (friendlist.js)

now we get to the view in question

• the region is still #app and it loads the share template with the selected model.

• there's a new variable 'amnestyMessage' declared.

• when the user clicks submit it takes whatever is in the text box and passed it to postToFacebook

• if it works, it renders the success view.

• I've tried manually destroying the amnestyApp.Views.share.destroy() in the onRender function of the success view but then nothing shows up. Why is that?

• I've also console.log(amnestyApp.Views.share.isDestroyed()); in the onRender function of the success view and it says it is "false" so it is still hanging around.

Why does explicitly destroying the view kill the whole thing? And why isn't it done automatically?

Perhaps I have started the whole thing off wrong? I don't think so: https://marionettejs.com/docs/master/marionette.region.html#defining-the-application-region

Maybe I need to do the second bit and explicitly get that app region and show the view in it:

 onStart: function() {
    var main = this.getRegion();  // Has all the properties of a `Region`
    main.show(new SomeView());
  }

Rather than rendering the hello view and the hello view using the el thing to take over that div. What do you think? Does any of that make any sense?

[robertocarroll]

So the opening would be something like:

onStart: function() {
    var main = this.getRegion();
    amnestyApp.Views.hello = new HelloWorld();
    main.show(amnestyApp.Views.hello);
   // amnestyApp.Views.hello.render();
  },

Getting rid of the render (commented out) and instead showing the view in the region.

[robertocarroll]

Actually, if I do the getRegion and tag it to the amnestyApp variable, I could replace all of the renders with amnestyApp.mainRegion.show

onStart: function() {
    amnestyApp.mainRegion = this.getRegion();
    amnestyApp.Views.hello = new HelloWorld();
    amnestyApp.mainRegion.show(amnestyApp.Views.hello);
   // amnestyApp.Views.hello.render();
  },

[robertocarroll]

See what you think of this:

https://github.com/wearethoughtfox/amnesty-facebook/commit/785405b7567ef8d04a1e28fb325bd241ddfcb597

(I probably should have done it in a branch and done a pull request). Anyway, now the console.log(amnestyApp.Views.share.isDestroyed()); in success.js returns true, which is good I think.

[paulwaitehomeoffice]

Marvellous — that seems to have fixed it, as far as I can see from the console messages.

[robertocarroll]

Oh cool. It's taking advantage of the Marionette regions properly, I think, which I wasn't doing at all earlier. One last thing would be that pesky

onNominateAgain: function(e) {
       amnestyApp.Views.myFriends.render();
    }

The friends view needs a render because it has child views etc. I think it will be ok because we actually want to re-use that view, but I'm just checking.

[robertocarroll]

I think we can close this now can't we?

[pauldwaite]

Absolutely, it looked fixed to me.