BP Nouveau: the very first activity is not injected into the stream.

[imath]

When it's the very first activity, the ul.activity-list.item-list.bp-list is missing into the markup, so we need to make sure the initial feedback message is replaced by this container on a successfull posted activity before injecting it in it :)

[boonebgorges]

I've confirmed the problem and the general approach of the fix, but it doesn't fix the problem when response.is_directory is false - for instance, on a group's Activity page. What's the purpose of the response.is_directory check? Can we just remove it?

[imath]

To be honest this long check is giving me headache !

I'm going to look at it very carefully. There's (at least) situations where it makes sense : for instance when you are on the mentions scope : it adds a success feedback but does not prepend the activity in the stream because, it's not a mention.

[imath]

@boonebgorges ​7774.2.patch is fixing it.

I've added inline comments to explain, why sometimes the activity is not added to the stream but a successful feedback is displayed instead.

[boonebgorges]

Thanks, @imath. The basic logic here seems OK to me.

A thought about your headache :) Currently, prepended is being determined in a procedural way, which results in lots of scattered code. I wonder if some of this might be handled in a more declarative way. Let's say that, for example, bp.Nouveau.Activity had a property or a method contexts or something like that, which might look something like:

contexts: {
  mentions: {
    prependNewItems: function() { something that tests whether the new item has `@me` }
  },
  my_groups: {
    prependNewItems: false
  }
  sitewide: {
    prependNewItems: true
  }
}

and so forth. We could also use this space to define things like template and object that are currently handled inline in objectRequest. This might help us simplify the internals (you just check getContext().prependNewItems or whatever) and make the whole thing easier to reason about and maintain. This is just a thought and is something that definitely doesn't need to happen for 3.0, but I thought I'd bring it up to see what you think.

Aside from this the patch looks fine :-D

[imath]

Thanks a lot for your feedback @boonebgorges i really like the idea 👍 I'll look at it tomorrow and if i feel we running out of time for 3.0, i'll commit current approach ;)

[imath]

@boonebgorges 7774.3.patch is my first attempt to convert your idea. I've added 2 "helpers" to check the activity is consistent with active filter and current search terms. Then there are 2 objects that corresponds to the "post in" select box (post in my profile = user, post in a group = group).

I've tested it and it seems to work as expected.

The only thing i have a doubt on are helper function names :)

[boonebgorges]

Thanks for working on this, @imath. 7774.3.patch​ certainly makes the inline toPrepend logic a lot easier to read and understand. But I'm afraid that the getContext() approach, as currently written, contains too much complexity and internal logic, and doesn't really capture what I was trying to suggest. For that reason, I'm recommending that we go with 7774.2.patch​ for the time being, and look at this in more detail in the future.

For the record, my idea is more like this:

1. The application always knows its currentPostContext (or something like that). These would be, at a glance, all, friends, groups, favorites, mentions, group. When initially setting up the application, this should be set (as part of setupGlobals() maybe), and whenever you click to a different tab, it should be changed.

2. Somewhere in the application we have a list of postContexts, each of which is an object that has some boolean/text properties (things like object and scope as they are sent to objectRequest()) but also callback support for things like prependNewItems. That is: instead of having a single prependNewItems() callback with a bunch of context-related logic inside of it, we have each context declare its own simple callback. Shared logic (like textContainsCurrentSearchTerm() or textContainsSelfMention() or whatever) can be broken into standalone methods.

3. Then the inline check is something like if ( currentContext.prependNewItems( text ) ), much like you've written it. We'd also use it when assembling objectRequest params, and maybe other stuff too.

I started to build this but I realize that the presence of the "Post in" dropdown, along with search and mention stuff, makes it non-trivial to implement. So instead of doing a mediocre job implementing my idea, let's just go with the more straightforward approach in 7774.2.patch​.

[imath]

Thanks a lot for your feedback @boonebgorges i'll go with 7774.2.patch, close this ticket and open a new one where we will be able to improve the getContext() approach.

[imath]

In 12038:

BP Nouveau: generate the Activity container before adding new entries

If the Activity container is missing, make sure to generate it before prepending new activities. If the container is available the activity will only be inserted if the activity stream can welcome it. For instance, an activity posted in a Private group will be prepended to the list only if the "My Groups" scope is active.

Fixes #7774