Status-react: Implement loading screen public chat

[hesterbruikman]

Extension to PR https://github.com/status-im/status-react/pull/7663 and #7454

Problem

A screen was introduced to replace the start screen of a public chat from ""There are no messages in this chat yet" to "It's been quiet here for the last 24h, start the conversation or share this chat."

This screen or a populated chat is only shown after all messages of the last 24h have been fetched. This can take up to seconds.

Implementation

Implement a loading screen to show between the public chat being opened and message fetching being completed.

[hesterbruikman]

@StatusSceptre can you reserve this bounty for @bitsikka is an additional unforeseen requirement to a bounty they were working on. https://github.com/status-im/status-react/pull/7663

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__This issue now has a funding of 80.0 DAI (80.0 USD @ $1.0/DAI) attached to it as part of the Status-im fund.__

• If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
  
  
  
  • Want to chip in? Add your own contribution here.
  
  
  • Questions? Checkout Gitcoin Help or the Gitcoin Slack
  
  
  • $42,937.78 more funded OSS Work available on the Gitcoin Issue Explorer
  
  

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__Work has been started__.

These users each claimed they can complete the work by 10 months, 3 weeks from now.
Please review their action plans below:

1) bitsikka has been approved to start work.

I will find correct subscription containing property that signals message fetching being completed. In fact that signal might be the existence and non nil value of :last-message-content in :chats/current-chat subscription.

I will change the logic of the conditions(that already includes checking whether messages are empty) that causes empty screen to be shown - specifically include in the condition of the existence of :last-message-content and it's value being nil - around here:

https://github.com/status-im/status-react/blob/539900b7bc100e566c1df61f7c399e129f1d80ab/src/status_im/ui/screens/chat/views.cljs#L187

Problem with checking for the existence of and nil value of :last-message-content is that it would work for new public chat being created or adding existing chat for the first time. However, if the user deletes a chat and then adds it back that condition won't work anymore. Unless, :last-message-content is dissociated from app-db(perhaps realm db as well) upon deactivation of a chat.

Not sure of any unintended consequences of dissociating :last-message-content at the moment. Or, whether this implementation approach is the best that can be done for the problem.

Any advice/comment/suggestions?

Thanks!

Learn more on the Gitcoin Issue Details page.

[hesterbruikman]

^ @yenda can you give any advice on implementation of the chat loading screen? As in what signal to listen so see if message fetching has been completed?

[bitsikka]

The approach I described earlier works only partially and we need something better.

While I've been sleuthing the whole time, and gathering a lot of contextual knowledge, so far, it is clear only that this problem is not trivial.

Because, as it turns out, fetching messages, determining whether there will be messages or not(waiting for messages to get populated and declare not empty, or declare empty), and mounting messages-view with appropriate state, does not quite happen linearly as it is assumable(and mistakenly thought of as trivial) on the surface.

Really need help of someone with more contextual knowledge.

[yenda]

so if I understand correctly this issue is about creating a version of the screen for when the messages are currently being fetched and there is no messages fetched yet is that correct @hesterbruikman ?

In that case @bitsikka first I would suggest that the appropriate state to be shown in the chat view should be computed in a subscription (i.e :state #{:empty :loading :messages}).
I would not rely on last-message as it is not a normalized field and is sometimes empty for instance with messages from extensions. (empty? messages) is more reliable.
Regarding message fetching have you checked the subscription :mailserver/fetching?? I think this is the best suited in that situation, it will be true as long as messages are being fetched.

[hesterbruikman]

so if I understand correctly this issue is about creating a version of the screen for when the messages are currently being fetched and there is no messages fetched yet is that correct @hesterbruikman ?

Exactly

[bitsikka]

so if I understand correctly this issue is about creating a version of the screen for when the messages are currently being fetched and there is no messages fetched yet is that correct @hesterbruikman ?

In that case @bitsikka first I would suggest that the appropriate state to be shown in the chat view should be computed in a subscription (i.e :state #{:empty :loading :messages}).
I would not rely on last-message as it is not a normalized field and is sometimes empty for instance with messages from extensions. (empty? messages) is more reliable.
Regarding message fetching have you checked the subscription :mailserver/fetching?? I think this is the best suited in that situation, it will be true as long as messages are being fetched.

@yenda good point on

"appropriate state to be shown in the chat view should be computed in a subscription".

I see what you mean.

I had looked at :mailserver/fetching?, but it does not seem to be complete enough for this purpose. Because, it only accounts for primarily checking :mailserver/pending-requests, and not whether :mailserver.callback/request-success fired, or whether there was :signals/signal-received, and whether mailserver said "mailserver.request.completed", and more.

Now, "mailserver.request.completed" via :signals/signal-received would've been good point for checking, if the mailserver signaled whether there is message(s) coming or not - but, correct me if I'm wrong, it doesn't.

There's also the situation where for every newly added public chat, there are two requests to mailserver, where one is not related to fetching messages for the topic. This also makes things a bit complicated.

So, as far as I can see, we're left to wait and check if there's going to be :transport/messages-received, if so, wait for messages to be :message/add(ed), and decide for sure whether the newly added public chat is going to have messages or not.

Otherwise, if we rely on (empty? messages) and :mailserver/fetching? we end up with cases where:

• initial screen is blank, then show empty screen, then if there is going to be messages, messages list
• or, even, empty, blank, empty, messages

Help!

[yenda]

@bitsikka to me mailserver/fetching? is exactly what loading... means, and once it is false you either show empty or messages state

[bitsikka]

@bitsikka to me mailserver/fetching? is exactly what loading... means, and once it is false you either show empty or messages state

@yenda yes, semantically loading... is what :mailserver/fetching? means (and it being false ought to signal that fetching is complete).

What I am saying is, in the current implementation of existing code base around it, mailserver/fetching? becomes false even before messages have been fetched completely. Which means, there is no problem when there aren't any messages being received from mailserver, but if there are messages to be fetched(or rather received), empty view is shown before messages are shown. Which I'm sure is not what @hesterbruikman wants.

We will need the implementation of :mailserver/fetching? to account for no-message confirmation or messages fetched response, before it is declared false and loading... is indeed done. Not just as it currently stands, where it assumes checking if there are :mailserver/pending-requests, and connectivity being good, are enough.

For your convenience, I'm quoting the subs and the effect that influences them here:

(re-frame/reg-sub
 :mailserver/pending-requests
 (fn [db]
   (get db :mailserver/pending-requests)))

(re-frame/reg-sub
 :mailserver/fetching?
 :<- [:mailserver/state]
 :<- [:mailserver/pending-requests]
 :<- [:mailserver/connecting?]
 :<- [:mailserver/connection-error?]
 :<- [:mailserver/request-error?]
 (fn [[state pending-requests connecting? connection-error? request-error?]]
   (and pending-requests
        (= state :connected)
        (pos-int? pending-requests)
        (not (or connecting? connection-error? request-error?)))))

(fx/defn process-next-messages-request
  [{:keys [db now] :as cofx}]
  (when (and
         (mobile-network-utils/syncing-allowed? cofx)
         (transport.db/all-filters-added? cofx)
         (not (:mailserver/current-request db)))
    (when-let [mailserver (get-mailserver-when-ready cofx)]
      (let [request-to (or (:mailserver/request-to db)
                           (quot now 1000))
            requests   (prepare-messages-requests cofx request-to)
            web3       (:web3 db)]
        (if-let [request (first requests)]
          {:db                          (assoc db
                                               :mailserver/pending-requests (count requests)
                                               :mailserver/current-request request
                                               :mailserver/request-to request-to)
           :mailserver/request-messages {:web3       web3
                                         :mailserver mailserver
                                         :request    request}}
          {:db (dissoc db
                       :mailserver/pending-requests
                       :mailserver/request-to)})))))

[bitsikka]

Having that said, I have now a working wip PR in the works, and I will be proposing it soon (probably as part of #7454).

It involves small time-out to wait for the possibility of incoming messages and feels hackish in a way, but should work well in normal network situations at least.

It also looks like it could be robust with a bit of extension to signal/transport/mailserver request/response context. Specifically, if the mailserver could respond with indication that, whether or not it will be broadcasting the network with messages related to topic for current chat in its request-complete payload.

[bitsikka]

It also looks like it could be robust with a bit of extension to signal/transport/mailserver request/response context. Specifically, if the mailserver could respond with indication that, whether or not it will be broadcasting the network with messages related to topic for current chat in its request-complete payload.

regarding ^that part, looks like we can compare LastEnvelopeHash for it(the indication) 🤔.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__Work for 80.0 DAI (80.0 USD @ $1.0/DAI) has been submitted by__:

1. @bitsikka

@StatusSceptre please take a look at the submitted work:

• PR by @bitsikka

---

• Learn more on the Gitcoin Issue Details page
• Want to chip in? Add your own contribution here.
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $57,752.80 more funded OSS Work available on the Gitcoin Issue Explorer

[gitcoinbot]

⚡️ A *Builder Of A Better World* Kudos has been sent to @bitsikka for this issue from @StatusSceptre. ⚡️ Nice work @bitsikka! Your Kudos has automatically been sent in the ETH address we have on file.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__The funding of 80.0 DAI (80.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @bitsikka.__

• Learn more on the Gitcoin Issue Details page
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $86,928.73 more funded OSS Work available on the Gitcoin Issue Explorer

[bitsikka]

@hesterbruikman we can close this