Delete sources as soon as API action succeeds

[eloquence]

Currently we wait for the next sync from the server before we complete a pending deletion, and show the deletion as being in progress until then. This was done to mitigate concurrency issues, e.g., with a sync in progress briefly reinstating a source after it was deleted in the client.

However, this behavior imposes a significant cognitive burden on journalists to pay attention as to whether their deletions have completed. This is not the case in the web-based interface, where deletions are instantaneous.

For busy news orgs, deletion is a very common operation, so speedier processing here would seem to have high user value.

We should consider revisiting this logic, and update the UI and the local client state more quickly, as soon as the server has successfully kicked off its asynchronous deletion job. We will have to guard against regressions (e.g., issues like #858 re-appearing).

User story

As a journalist, I want to treat deletion as much as a "fire and forget" operation as possible, and not have to wait until the next successful sync.

[sssoleileraaa]

Right now you can delete many sources at the same time, but you'll see the pending deletion message until the source has been confirmed to be deleted by the server. This is because (1) we sync on a separate thread and (2) the server returns all sources and submissions for every sync; if a source is not in the sync response then it has been deleted on the server, and if a source that doesn't exist client-side is in the response then it's a new source on the server (this is where the bug can occur).

We've talked about doing a partial sync/ changes feed, which would entirely change the contract between the server and client, see freedomofpress/securedrop#5104 (comment). Aso, just as a reminder, we also have #1157 in the near-term backlog.

For now, our solution is to not delete information locally until a following sync confirms the server is updated to match the client request. One thing we could do to make the client more snappy is to remove the deleted information immediately from the UI upon request as soon as the job has been processed. Personally, I would prefer the "pending deletion" state until I know it's been removed from the local db, because it's also easy to ignore the sources that have pending deletions. This is still debatable.

[sssoleileraaa]

Also just linking this here in case it's relevant later: #1040

[sssoleileraaa]

In case it's helpful, here is a screenshot where you can see that I have two pending source deletions and am able to read a totally different conversation from another source while that's going on in the background. There is only a 15-second period between syncs so those sources will be removed soon.

[eloquence]

Thanks for the recap @creviera. This came up today in the context of the Safe Deletion feature (#1202) since we'll encounter similar questions there, and were wondering if we want to replicate the pattern for source deletion, or do something different. To recap, the Safe Deletion feature will give journalists the option to wipe files/messages without deleting the source account.

Regarding source deletion, so far we've had only one user express frustration with the current perceived slowness, though we have not systematically polled pilot participants about it.

The main concern with the current behavior is that it suggests that the operation is not successful until the "in progress" indicator is removed. This means we're essentially placing a lock on the journalist's attention until the next sync (which itself can take a lot longer than 15 seconds, depending on how many sources are on the server).

Yes, the user can switch to another source, but they will want assurance that the source is actually gone -- so they probably won't exit the app, and won't be able to fully cognitively switch to other work, until the "in progress" indicator is removed.

One thing we could do to make the client more snappy is to remove the deleted information immediately from the UI upon request as soon as the job has been processed.

By "job has been processed", do you mean that the API request has completed successfully? If so, I agree; that would IMO be significantly preferable over the current behavior. I wonder if we can use timing information from the server to avoid the kind of bugs we've seen before, where sources get re-created (i.e. if a metadata sync is older than the last API request that altered the server state, it can arguably be safely ignored).

[sssoleileraaa]

By "job has been processed", do you mean that the API request has completed successfully?

Yup!

If so, I agree; that would IMO be significantly preferable over the current behavior.

I'm still on the fence. For clarity, the two things we're comparing here are whether we should show the pending state until the information has been removed from the db locally VS show the pending state until the job (or batch job) is finished processing with a successful response. My gut tells me never lie to users 🙃 so only show the deletion operation as successful once it's been removed locally. But you could argue that it's not a lie because removing the source locally is more of a cleanup operation that we don't need to concern the user with.

I wonder if we can use timing information from the server to avoid the kind of bugs we've seen before, where sources get re-created (i.e. if a metadata sync is older than the last API request that altered the server state, it can arguably be safely ignored).

Yeah, ideally we would delete the source locally first. I don't think we'd need timing info if we kept a pending deletion table the same way we have a pending replies table. This would allow us to avoid the "ghost" situation. If we end up implementing a changes feed for Journalist API v2 (freedomofpress/securedrop#5104), let's say for the /users endpoint, then we might not need to do this. For instance if we only request new or deleted users every sync, then we wouldn't have a ghost situation. And our error handling should take care of concurrency issues.

For multi-delete, I don't think we need to worry about v2 or adding a pending-deletion table, but it's good to have more people thinking about this and weighing in.

[eloquence]

We (@creviera @ninavizz @rmol @zenmonkeykstop @emkll @eloquence) discussed this further today in the context of Safe Deletion (#1202).

We may be able to make some tweaks as we finish the first iteration of #1202:

• kick off the next sync immediately after deletion job success to reduce the delay
• improve the UX of the pending state (e.g., add spinner, graphics; @ninavizz is working on a revised design proposal we'll discuss next week)

We may revisit this issue later, possibly after the implementation of a changes feed API (as mentioned by @creviera above), which would benefit many operations other than deletion as well.

[sssoleileraaa]

Sounds good to me. It makes sense to keep this issue open for its context around how we can make deletion even faster if we delete the source locally outside of a sync once we're able to solve the issue of concurrency and ghosts in a new way. I'm not sure we'll want to, but we can revisit this later after/during implementation of v2. The time it takes to sync with the server after making a deletion request might be fast enough after v2 that we decide not to implement extra complexity for smaller gains.

[sssoleileraaa]

@conorsch pointing you here in case this helps your debugging of #1285. Since we don't yet have a changes feed the client waits until the server returns what it expects after a user-initiated operation. Ideally, we'd be able to delete a source or conversation locally, immediately after we get a 200 from the server, but we have to watch out for stale sync data happening on a separate thread that could assume the source or conversation in the stale sync data is new and needs to be created locally (and then later, when the next sync catches up it deletes the records from the local client db). This is what we've been referring to as the ghost issue, and there are different ways to avoid it. For instance, for starring we keep track of a pending toggle operation during the current session and ignore the sync data until it matches. Personally, I'd like to focus most of our efforts on a changes feed so am okay with storing pending state in the current session/ dropping stale syncs -- whatever is the simplest approach until we implement a changes feed.

[eloquence]

Duplicate of #1344.