delete source widgets for current sync update

[sssoleileraaa]

Description

• Fixes Source list doesn't update right away when you delete multiple sources #1113
• Fixes issue 8 documented here: Potential crashes between source deletion and accessing source in the gui #906 (comment)
  • Fixes client crashes when sources are reordered #1116
  • No STR for the second half
• Fixes issue 1 documented here: Potential crashes between source deletion and accessing source in the gui #906 (comment)
  • Fixes add-source scheduler crashes app because of unhandled exception #1117
  • No STR for the second half
  • While working on this fix, a side effect/refactor I made was to no longer delete widgets on behalf of a different ongoing sync running on a separate thread since the ongoing sync will delete the widgets shortly after.

Test Plan

• Verify Source list doesn't update right away when you delete multiple sources #1113 is fixed by following the STR there
• Verify add-source scheduler crashes app because of unhandled exception #1117 is fixed by following the STR there
• Verify client crashes when sources are reordered #1116 is fixed by following the STR
• Verify that we surround all access of db object properties in try-catch blocks in the SourceList.update method to confirm second half of issue 1 and 8 documented here: Potential crashes between source deletion and accessing source in the gui #906 (comment)
• Verify that test test_SourceList_update_when_source_deleted passes and that the rewrite makes sense (because if this makes sense then the refactor I made will make sense)

Regression test

1. run make dev
2. run the client
3. log into the Journalist Interface
4. get a source ready for deletion by having the deletion popup confirmation in a separate window
5. have a message ready to that source
6. (you can do this without adding a sleep in the code if you act fairly fast) delete the source and send the message right after
7. see that the message fails to send and that the source widget and conversation are deleted next sync (this is also the current behavior)

Checklist

If these changes modify code paths involving cryptography, the opening of files in VMs or network (via the RPC service) traffic, Qubes testing in the staging environment is required. For fine tuning of the graphical user interface, testing in any environment in Qubes is required. Please check as applicable:

• I have tested these changes in the appropriate Qubes environment
• I do not have an appropriate Qubes OS workstation set up (the reviewer will need to test these changes)
• These changes should not need testing in Qubes

If these changes add or remove files other than client code, the AppArmor profile may need to be updated. Please check as applicable:

• I have updated the AppArmor profile
• No update to the AppArmor profile is required for these changes
• I don't know and would appreciate guidance

If these changes modify the database schema, you should include a database migration. Please check as applicable:

• I have written a migration and upgraded a test database based on master and confirmed that the migration applies cleanly
• I have written a migration but have not upgraded a test database based on master and would like the reviewer to do so
• I need help writing a database migration
• No database schema changes are needed

[redshiftzero]

Reproduced #1113 #1116 #1117 on master branch, confirming that they no longer result in crashes (for #1116 #1117) and that the widget deletion occurs in a single sync period for #1113 with the changes on this branch ✨ . A couple of thoughts/findings inline, lmk what you think!

Dismissing my review so other folks can take over review. Most of my comments were addressed already!

[sssoleileraaa]

Even though everything is green and passes, I'd like to work on adding regression tests around widgets not getting added to the cache to prevent the issue that @redshiftzero found earlier from happening again: #1114 (review)

[sssoleileraaa]

Dismissing my review so other folks can take over review. Most of my comments were addressed already!

I believe at this point, I've addressed all of @redshiftzero's review comments. So this is ready for a second pass.

[kushaldas]

Tested with the following:

• Verify #1113 is fixed by following the STR there
• Verify #1117 is fixed by following the STR there
• Verify #1116 is fixed by following the STR
• Verify that we surround all access of db object properties in try-catch blocks in the SourceList.update method to confirm second half of issue 1 and 8 documented here: #906 (comment)
• Verify that test test_SourceList_update_when_source_deleted passes and that the rewrite makes sense (because if this makes sense then the refactor I made will make sense)