Detach commit statuses from pull requests

[niik]

Overview

References #6982

Description

This is the second step in a series of PRs to address how the app retrieves, stores, and presents pull requests (see previous work).

This PR removes the direct relationship between pull requests and commit statuses. Prior to this change all pull requests in the app had a commit status attached to them and when we loaded the list of PRs we used the SHA provided in the PR details from the API to look up the commit status. This meant, among other things, that the commit status would not refresh until we refreshed the list of PRs (so that we got the latest SHA). With this decoupling we are now asking the API for the latest commit status by querying the named ref (i.e refs/pulls/123/head) instead of providing an explicit SHA such that we're always getting the commit status for the most recent commit of any particular PR.

This decoupling also means that we no longer have to wait for the commit statuses to load in order to show the list of open PRs, this significantly cuts down on the time it takes to display the PR list from a cold cache.

The approach I've taken here is not one that we've used a whole lot in the past but I think the advantages it provides are more than sufficient to merit it. I've introduced a new store; the CommitStatusStore. Through this store consumers are able to subscribe to a particular ref (can be a named ref or a SHA) and receive updates for that ref until they choose to unsubscribe. This means that the CIStatus component is able to subscribe directly to the store when it's mounted (and unsubscribe when it's unmounted) meaning that we'll now load statuses on-demand as the component appears on screen (with overrun/underrun provided by our list component meaning that it'll load a few statuses before they're scrolled to. This works really well as we're not relying on commit statuses for any application logic, only presentation. Should that change the store can be expanded to provide an awaitable resource to get the latest status for any given ref.

The store will run up to 5 concurrent fetches of status from the API (compared to the previous implementation which did one status at a time serially) which greatly improves performance and minimizes effects of any single status fetch taking a long time to get. The concurrency management is handled by an external library, p-limit but in addition to that the store will re-use handle multiple subscriptions to the same ref and ensure that only one fetch is run for each. Additionally the store handles unsubscriptions from a ref during refresh such that switching a repository will stop refreshing refs that are no longer subscribed to. A ref will be fetched at most once a minute but there's a three-minute interval timer that runs while the app is focused and the store will also refresh when the app regains focus.

Statuses are cached in memory (I've removed the database table that was used when statuses were tied to PRs) in a least recently used cache (quick-lru) to prevent unbounded memory growth.

PR loading screen

Prior to this PR we had a facade "skeleton" loading screen for PRs. That pattern only existed in this particular view and rather than investing time making that compatible with the new store I've switched over to a loading view more in line with what all our other loading lists do (such as the cloneable repository list).

Notes for testing / backwards incompatibility

This PR deletes a table from the pullRequestsDatabase making it incompatible with prior versions of GitHub Desktop. As such, after running this branch you need to manually delete the PR database to have it recreated again before you can use PRs in development or other branches. Do this by opening DevTools, clicking on the Applications tab, locating the PullRequestsDatabase under the IndexedDB group in the sidebar and click Delete database

Release notes

Notes: [Improved] Pull requests load faster and PR build status updates automatically.

[tierninho]

Tested and overall functions well. Super fast and responsive.

• spotted this, which seemed to happen organically
  
• Is this part of this PR? I was wondering if any testing needs to be done around this (if it's new)? skipping fast-forward for all branches as there are 21 local branches - this will run again when there are less than 20 local branches tracking remotes

[niik]

Tested and overall functions well. Super fast and responsive.

💖

• spotted this, which seemed to happen organically

While not related to this PR that's an interesting error and I've opened #7169 with more context about that.

That error is actually thrown by fetchCommit which is used to retrieve user information for a particular commit in order to display the correct name and avatar.

• Is this part of this PR? I was wondering if any testing needs to be done around this (if it's new)?

It's not new nor part of this PR. It was introduced in #4447.

[shiftkey]

I wanted to write up this interesting case I stumbled upon - just wanted to flag this in case it's intentional.

Here's what I did:

• walked through the steps to create screenshots for differentiate draft PRs from normal PRs in the PR list #7170
• closed laptop to suspend machine without exiting Desktop
• relocated to different location (no internet)
• unlocked machine and switched back to Desktop

I was hoping that the cached values would be shown here, but it looks like - because the network requests failed while trying to refresh the list - the previous values are cleared from the list:

There's mention in the PR about an LRU cache in-memory rather than using IndexedDB, which is fine, but I'll dig into this as part of the PR to see if I can find the specific change related to this behaviour...

[shiftkey]

These changes look great, and it's very cool to see us be able to clean up a bunch of code that's no longer needed now we've simplified this area.

After a bunch of testing most of the feedback I have is on minor stuff, happy to discuss what's worth punting or deferring...