Remove coupling of allDiscussions in the issues discussion tab counter

What does this MR do?

Resolves some tech debt as raised in #32898 (closed). Specifically, allDiscussions has been decoupled from discussionTabCounter.

Previously:

Previously, changes to allDiscussions were triggering an update of the DOM element that displays the discussion counter, which represents the text that should be displayed to the user.

Now:

Now, a computed value that depends on only isLoading and discussionTabCounter has been created. This computed value is watched, and the DOM gets updated in the watcher.

Why is this change necessary?

While allDiscussions is tangentially related to discussionTabCounter, there is no explicit relationship between them, and therefore unnecessary complexity and fragility is introduced by this dependency.

Why not just watch on discussionTabCounter instead?

The discussion counter relies on 2 pieces of data: discussionTabCounter, and isLoading. Unfortunately, due to the nature of Vuex, if we watch on discussionTabCounter and check for isLoading, we will have problems; after we fetchDiscussions, the discussionTabCounter is computed before isLoading is set (via setLoadingState() action), and therefore, even though in reality we have finished loading, isLoading is still true within the watcher's logic, and the DOM won't get updated.

Okay, why not just watch on discussionTabCounter instead, and remove the check for isLoading?

Due to the organisation of the logic preceding this (i.e. fetching discussions etc.) this will actually works, but it's very fragile. If any code ever changes, we could get undesirable flashes of numbers while we're loading (i.e., 0, then after load, 12)

If isLoading is set to true while the user is on the page, then won't we get flashes of nothing, which is undesirable?

Yes, but (as far as I can tell) this never happens; isLoading is only used for initial load.

Will the tab counter be set to 0 for a new issue?

Yes. This is because the computed value depends on both isLoading and discussionTabCounter. If it was just discussionTabCounter, there's a chance it might not work. This is because watchers only get triggered when a value or reference changes e.g., if a watched value is 0, and the computed property is forced to update, but the value is still 0, then the watcher won't get triggered. As an aside, if the property was first [0], and was recomputed as [0], the watcher would be triggered (TIL!).

So because the computed property also depends on isLoading, it will force an update through every time, including the initial load on an issue with 0 discussions

Edited by 🤖 GitLab Bot 🤖

Merge request reports

Loading