From fc40aa7c8eb5bb18fdb2dae184feaa76b1b38ce5 Mon Sep 17 00:00:00 2001
From: rc-swag <58423624+rc-swag@users.noreply.github.com>
Date: Tue, 7 Jan 2025 22:32:02 +1000
Subject: [PATCH] feat(windows): address review comments

---
 .../main/Keyman.System.RemoteUpdateCheck.pas  | 17 +++---
 .../main/Keyman.System.UpdateStateMachine.pas | 52 ++-----------------
 2 files changed, 11 insertions(+), 58 deletions(-)

diff --git a/windows/src/desktop/kmshell/main/Keyman.System.RemoteUpdateCheck.pas b/windows/src/desktop/kmshell/main/Keyman.System.RemoteUpdateCheck.pas
index 9e4b0b007e2..c78060bbfd9 100644
--- a/windows/src/desktop/kmshell/main/Keyman.System.RemoteUpdateCheck.pas
+++ b/windows/src/desktop/kmshell/main/Keyman.System.RemoteUpdateCheck.pas
@@ -56,8 +56,6 @@   TRemoteUpdateCheck = class
     property ShowErrors: Boolean read FShowErrors write FShowErrors;
   end;
 
-procedure LogMessage(LogMessage: string);
-
 (**
   * This function checks if a week or CheckPeriod time has passed since the last
   * update check.
@@ -73,11 +71,13 @@ implementation
   System.Win.Registry,
   Winapi.Windows,
   Winapi.WinINet,
+  Sentry.Client,
 
   GlobalProxySettings,
   KLog,
   keymanapi_TLB,
   KeymanVersion,
+  Keyman.System.KeymanSentryClient,
   Keyman.System.UpdateCheckStorage,
   kmint,
   ErrorControlledRegistry,
@@ -103,7 +103,8 @@ constructor TRemoteUpdateCheck.Create(AForce: Boolean);
 destructor TRemoteUpdateCheck.Destroy;
 begin
   if (FErrorMessage <> '') and FShowErrors then
-    LogMessage(FErrorMessage);
+        TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_ERROR,
+      '"+FErrorMessage+"');
 
   KL.Log('TRemoteUpdateCheck.Destroy: FErrorMessage = ' + FErrorMessage);
   KL.Log('TRemoteUpdateCheck.Destroy: FRemoteResult = ' +
@@ -223,13 +224,6 @@ function TRemoteUpdateCheck.DoRun: TRemoteUpdateCheckResult;
   end;
 end;
 
-// temp wrapper for converting showmessage to logs don't know where
-// if nt using klog
-procedure LogMessage(LogMessage: string);
-begin
-  KL.Log(LogMessage);
-end;
-
 function ConfigCheckContinue: Boolean;
 var
   Registry: TRegistryErrorControlled;
@@ -260,7 +254,8 @@ function ConfigCheckContinue: Boolean;
     on E: ERegistryException do
     begin
       Result := False;
-      LogMessage(E.Message);
+          TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_ERROR,
+      E.Message);
     end;
   end;
 end;
diff --git a/windows/src/desktop/kmshell/main/Keyman.System.UpdateStateMachine.pas b/windows/src/desktop/kmshell/main/Keyman.System.UpdateStateMachine.pas
index 71fbdfb822b..8a5642091a5 100644
--- a/windows/src/desktop/kmshell/main/Keyman.System.UpdateStateMachine.pas
+++ b/windows/src/desktop/kmshell/main/Keyman.System.UpdateStateMachine.pas
@@ -637,7 +637,7 @@ procedure UpdateAvailableState.StartDownloadProcess;
   FResult: Boolean;
   RootPath: string;
 begin
-  // call seperate process
+  // call separate process
   RootPath := ExtractFilePath(ParamStr(0));
   FResult := TUtilExecute.ShellCurrentUser(0, ParamStr(0),
     IncludeTrailingPathDelimiter(RootPath), '-bd');
@@ -746,8 +746,10 @@ procedure DownloadingState.Enter;
 
   if (not DownloadResult) then
   begin
-    // Failed three times in this process return to the
-    // IdleState to wait 7 days before trying again
+    // Failed three times in this process; return to the
+    // IdleState to wait 'CheckPeriod' before trying again
+    TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_ERROR,
+    'Error Updates not downloaded after 3 attempts');
     ChangeState(IdleState);
   end
   else
@@ -1016,48 +1018,4 @@ procedure InstallingState.HandleFirstRun;
   // Result := kmShellContinue;
 end;
 
-// Private Functions:
-function ConfigCheckContinue: Boolean;
-var
-  Registry: TRegistryErrorControlled;
-begin
-  { Verify that it has been at least CheckPeriod days since last update check }
-  Result := False;
-  try
-    Registry := TRegistryErrorControlled.Create; // I2890
-    try
-      if Registry.OpenKeyReadOnly(SRegKey_KeymanDesktop_CU) then
-      begin
-        if Registry.ValueExists(SRegValue_CheckForUpdates) and
-          not Registry.ReadBool(SRegValue_CheckForUpdates) then
-        begin
-          Result := False;
-          Exit;
-        end;
-        if Registry.ValueExists(SRegValue_LastUpdateCheckTime) and
-          (Now - Registry.ReadDateTime(SRegValue_LastUpdateCheckTime) >
-          CheckPeriod) then
-        begin
-          Result := True;
-        end
-        else
-        begin
-          Result := False;
-        end;
-        Exit;
-      end;
-    finally
-      Registry.Free;
-    end;
-  except
-    { we will not run the check if an error occurs reading the settings }
-    on E: Exception do
-    begin
-      Result := False;
-      LogMessage(E.Message);
-      Exit;
-    end;
-  end;
-end;
-
 end.