implemented draft feature, including attatchments #225

clemensott · 2021-02-27T12:12:26Z

Implemented the saving and loading of drafts including the attatchemnt file.
Fixed the saving of color from conversation.

I'm not sure if my migration is correct, because I just copied m6 by hand and renamed it to m7 and added the new property DraftFileTokens.

Signal-Windows.Lib/Models/SignalConversation.cs

clemensott · 2021-02-27T12:21:12Z

Signal-Windows.Lib/Storage/DB.cs

+            }
+        }
+
+        public static void InsertOrUpdateConversationsLocked(IEnumerable<SignalConversation> conversations)


I added this function to improve the performance when saving all conversations on suspending.

Signal-Windows.Lib/Storage/DB.cs

clemensott · 2021-02-27T12:32:11Z

Signal-Windows/Controls/Conversation.xaml.cs

+             * because on mobile app gets supended during file picking and
+             * the new conversation does not have the DraftFileTokens
+             */
+            if (conversationThreadIdChanged)


I thought the best approach to solve this issue is to just keep the current draft if its the same thread id and set the kept draft in the new conversation.

Signal-Windows/ViewModels/MainPageViewModel.cs

clemensott · 2021-02-27T12:44:25Z

Signal-Windows/Controls/Conversation.xaml.cs

+                    Task.Run(() =>
+                    {
+                        SignalDBContext.InsertOrUpdateConversationLocked(SignalConversation);
+                    });


If the token is not saved in the database right away it could happen, that StorageApplicationPermissions and database get out of sync when the app crashes or gets in other way suspended without saving.

clemensott · 2021-02-27T12:51:44Z

Signal-Windows/App.xaml.cs

+            await Task.Run(() =>
+            {
+                SignalDBContext.InsertOrUpdateConversationsLocked(Views.Values.SelectMany(view => view.Locator.MainPageInstance.Conversations));
+            });


I'm not sure if this is the best position to save the current state of all conversations. Maybe within Handle.Release()? But I think in Handle.Release() is no nice way to access the conversations.

A question I have is what in the conversation state isn't saved when the app shuts down. You're already saving the attachment state in the DB immediately after selection. The draft text may be a little harder as we wouldn't want to save to the DB on every keystroke. I could see that being saved in App_Suspending

With this code in App.xaml.cs everything is saved in the conversation state on App_Suspending. In Conversation.xaml.cs the state is only saved, so that no files are saved in the FutureAccessList, which then might not be saved in the state or database, because the app crashes or similar.

I think I have better idea now. We could save the selected file in FutureAccessList and the state of the current conversation in the database on SaveDraftInCurrentConversation(). With this approch saving the state of all conversations in App_Suspending is not needed, because the only conversation that has to be saved is the current and this gets done by Handle.Release().

golf1052 · 2021-03-05T05:49:03Z

I'm not sure if my migration is correct, because I just copied m6 by hand and renamed it to m7 and added the new property DraftFileTokens.

If you used the Add-Migration command in the Package Manager Console in Visual Studio you did the migration correctly. I should add documentation to the repo on doing DB migrations.

clemensott · 2021-03-05T18:52:37Z

I tried the Add-Migration command (Add-Migration m7 -Context SignalDBContext) but I get an error:
Startup project 'Signal-Windows.Lib' is a class library. Select a Universal Windows Platform app as your startup project and try again.

clemensott · 2021-03-05T19:34:21Z

I think I found the problem. I nedded to select the project Signal-Windows and add -Project Signal-Windows.Lib the command. I created an PR to fix the documentation.

I updated/fixed the migration of this PR.

golf1052 · 2021-03-09T09:50:06Z

Signal-Windows/App.xaml.cs

+            await Task.Run(() =>
+            {
+                SignalDBContext.InsertOrUpdateConversationsLocked(Views.Values.SelectMany(view => view.Locator.MainPageInstance.Conversations));
+            });


A question I have is what in the conversation state isn't saved when the app shuts down. You're already saving the attachment state in the DB immediately after selection. The draft text may be a little harder as we wouldn't want to save to the DB on every keystroke. I could see that being saved in App_Suspending

golf1052 · 2021-03-09T10:02:21Z

Signal-Windows/Controls/Conversation.xaml.cs

+            }
+            catch (Exception e)
+            {
+                Logger.LogError("SetSelectedFile() failed: {0}\n{1}", e.Message, e.StackTrace);


Did this fail for you while testing? Does it make sense to fail to save the selected file in the StorageApplicationPermissions and the DB but still show it in the UI?

Actually no, I just like to put everything that has to do with IO in a try catch. I change it.

golf1052 · 2021-03-09T10:03:51Z

Signal-Windows/Controls/Conversation.xaml.cs

+                if (SelectedFile == null)
+                {
+                    AddedAttachmentDisplay.HideAttachment();
+                }
+                else
+                {
+                    AddedAttachmentDisplay.ShowAttachment(SelectedFile.Name);
+                }
+                UpdateSendButtonIcon();


I feel like this should be split out into a separate method called SetSelectedFileInUI. Then the only two places where the argument save is set to false could just directly call the new method and the save argument could be removed.

clemensott

I changed when the conversations and it's draft get saved. It's now saved in SaveCurrentConversationInDatabase(), which is call on change of conversation and shutdown of app. SelectedFile gets save in FutureAccessList there aswell.

Obviously the draft of the current conversation is not saved when the app crashes but I think this is acceptable, as long as the DraftFileTokens in the database don't get out of sync with FutureAccessList, which is case.

clemensott · 2021-03-09T11:43:23Z

Signal-Windows/App.xaml.cs

+            await Task.Run(() =>
+            {
+                SignalDBContext.InsertOrUpdateConversationsLocked(Views.Values.SelectMany(view => view.Locator.MainPageInstance.Conversations));
+            });


With this code in App.xaml.cs everything is saved in the conversation state on App_Suspending. In Conversation.xaml.cs the state is only saved, so that no files are saved in the FutureAccessList, which then might not be saved in the state or database, because the app crashes or similar.

I think I have better idea now. We could save the selected file in FutureAccessList and the state of the current conversation in the database on SaveDraftInCurrentConversation(). With this approch saving the state of all conversations in App_Suspending is not needed, because the only conversation that has to be saved is the current and this gets done by Handle.Release().

clemensott commented Feb 27, 2021

View reviewed changes

Signal-Windows.Lib/Models/SignalConversation.cs Show resolved Hide resolved

clemensott force-pushed the fix_add_attchment_mobile branch from 2767ef7 to c00dba4 Compare February 27, 2021 12:27

implemented draft feature, including attatchments

9189943

clemensott force-pushed the fix_add_attchment_mobile branch from c00dba4 to 9189943 Compare February 27, 2021 12:34

clemensott marked this pull request as ready for review February 27, 2021 12:48

clemensott commented Feb 27, 2021

View reviewed changes

fixed migration

67a0dbc

clemensott mentioned this pull request Mar 5, 2021

Contact color always reset back to original color #226

Closed

golf1052 reviewed Mar 9, 2021

View reviewed changes

golf1052 and others added 2 commits March 9, 2021 02:14

Merge branch 'master' into pr/225

447386d

changed when conversation gets saved in database

d9eb53e

clemensott commented Mar 9, 2021

View reviewed changes

clemensott requested a review from golf1052 March 9, 2021 13:09

golf1052 approved these changes Mar 9, 2021

View reviewed changes

golf1052 added 2 commits March 9, 2021 22:11

Fix async not waited warning

9671c25

Merge branch 'master' into pr/225

6a9ef68

golf1052 merged commit ce75302 into signal-csharp:master Mar 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implemented draft feature, including attatchments #225

implemented draft feature, including attatchments #225

clemensott commented Feb 27, 2021 •

edited

clemensott Feb 27, 2021

clemensott Feb 27, 2021

clemensott Feb 27, 2021

clemensott Feb 27, 2021

golf1052 Mar 9, 2021

clemensott Mar 9, 2021

golf1052 commented Mar 5, 2021

clemensott commented Mar 5, 2021

clemensott commented Mar 5, 2021

golf1052 Mar 9, 2021

golf1052 Mar 9, 2021

clemensott Mar 9, 2021 •

edited

golf1052 Mar 9, 2021

clemensott left a comment

clemensott Mar 9, 2021

implemented draft feature, including attatchments #225

implemented draft feature, including attatchments #225

Conversation

clemensott commented Feb 27, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

golf1052 commented Mar 5, 2021

clemensott commented Mar 5, 2021

clemensott commented Mar 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clemensott Mar 9, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clemensott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clemensott commented Feb 27, 2021 •

edited

clemensott Mar 9, 2021 •

edited