bcgov / business-create-ui

BC Registry Services - Legal Entities - Create Incorporation Application
Apache License 2.0
7 stars 48 forks source link

21110 Non-functional code to delete unsaved Minio documents when UI exits #709

Closed severinbeauvais closed 5 months ago

severinbeauvais commented 5 months ago

Issue #: bcgov/entity#21110

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the bcrs-entities-create-ui license (Apache 2.0).

severinbeauvais commented 5 months ago

All - this is a non-functional solution but I learned some things along the way and wanted to share.

Here are some comments I added to the ticket:

Part 2 Summary

After a fair bit of research and testing, I believe this cannot be implemented, as there is no way to delay the app from unloading while any unsaved documents are deleted.

Specifically, window.onbeforeunload can only be used to display a generic (synchronous) "unsaved changes" dialog. The next event that we can catch is window.onpagehide (because onunload is deprecated) and it's synchronous, too. So, we cannot wait for any network calls (deletions) to complete, and in fact any pending network calls are cancelled when the app terminates.

Basically, asking the client to clean up before exiting is not a good design (and it doesn't handle all cases, such as power interruptions and some mobile scenarios). According to what I've researched, a better design is to have the API auto-delete any documents whose keys we haven't stored in the filing JSON. I have created #21728 for this.

severinbeauvais commented 5 months ago

One potential client-side solution is to use a Service Worker to delete unsaved files. This is a separate thread and apparently can be made to outlive the initial web page thread. However, this still doesn't handle all cases and it's complex/overkill for just this purpose.

JazzarKarim commented 5 months ago

So what's the alternative here Sev? Do you have something in mind?

severinbeauvais commented 5 months ago

So what's the alternative here Sev? Do you have something in mind?

We can often detect when the web app ends (eg, close tab, close browser, navigate away) but there are some undetectable conditions (like power loss, and some mobile scenarios).

In the end, it's not a good design for the client (web app) to clean up what are essentially temporary documents on the (Minio) server. We should assume the client could just "disappear" and the servers need to protect themselves, including their data integrity, which includes cleaning up orphaned documents. (I call them temporary documents because they are not "locked in" until the filing is submitted.)

A better design would have the server do the cleanup. Legal API is already involved in getting a key for the documents that the UI uploads. Legal API could hold on to those keys and delete documents that don't appear in a filing after, say, a year. (Or also store the filing id and then purge obsolete documents when that filing is submitted.)

cc: @vysakh-menon-aot