FEATURE: improve blank page syndrome on the user activity pages (PR #14311)

This improves blank page syndrome on the next pages:

  • activity
  • activity/replies
  • activity/drafts
  • activity/likes-given

An example:

We still show this when a user is looking to the page of another user:

We were sending a string key to the server:

And the server was just adding to it .self or .other suffix and then returning the key back to the client:

There is no need to be doing it now. I’ve fixed it on the client side in this PR, and I’m going to remove this code from the server method in a subsequent PR.

GitHub

Personally, I would extract this function into the User class. Something like User.isCurrentUser(user)

I do have one comment but the rest of the changes look good to me.

I’ll be happy to change it to another function if there are pros in another implementation, or if there are some problems with my implementation.

By now, I prefer my version. Here is why.

A user may look at his own page or at a page of another user. Very often, we need to know if a user looks at a page of another user. Take a look how the name of my function is explicitly saying just exactly what we’re checking. Explicit descriptive names are very important.

Moreover, we check not only if this is the current user, we actually need to check two things:

  • if this is an anonymous user (for an anonymous user, any page is a page of another user)
  • and only if it’s not an anonymous user, we check if it is his page

I would even implement this function like this, more explicitly, I would extract a variable to make my intentions obvious:

  isAnotherUsersPage(user) {
    const isAnonimousUser = !this.currentUser;
    if (isAnonimousUser) {
      return true;
    }

    return user.username !== this.currentUser.username;
  },

Only this part of the function can go to User.isCurrentUser(user), not 100% sure if we need to extract it, but maybe yes:

  isCurrentUser(user) {
    return user.username !== this.currentUser.username;
  }

By the way, if we extract that part, my function will look better

  isAnotherUsersPage(user) {
    const isAnonimousUser = !this.currentUser;

    return isAnonimousUser || User.isCurrentUser(user)
  },

@tgxworld What do you think? Also, maybe there are some problems with my function that I don’t see?

Thanks for the detailed explanation again :slight_smile: The last example looks good to me actually. Actually I wonder if we already have such a function lying around somewhere.

Actually I wonder if we already have such a function lying around somewhere. I feel now like I saw something like this, going to look for it.

I feel now like I saw something like this, going to look for it.

I had a similar use case today but couldn’t find it. Anyway, please ignore my comments here because I realised that the currentUser object is not injected into models automatically.

@tgxworld I applied to this PR the change you suggested in another PR. It’s a very good point that we shouldn’t mutate rest models in route model hooks.