Don't use integers in place of booleans in RPC

[lighterowl]

A lot of places in transgui use integers in RPC requests where the daemon expects booleans.

A recent commit in Transmission started actually enforcing this as the JSON parsing implementation has changed.

Originally reported in https://github.com/xavery/transgui/issues/79 and cherry-picked from https://github.com/xavery/transgui/commit/409512f85001d0865dba2a21ce1497b2c60c0d8d.

[what-the-diff[bot]]

PR Summary

• Simplified Logic in btTestPortClick Procedure Changes were made to the logic that checks if a port is open. It's now a simple truth check, rather than a not-zero check. This change makes the code easier to understand and maintain.

• Locked Start Button during Test Execution The Start button on the form is now disabled promptly after a testing process starts. This helps in preventing users from accidentally initiating another test process while one is already running.

• Re-enabled Start Button Post Test and After Checking Status The Start button is now promptly re-enabled after the testing process has completed, or when the status check procedure is called. This ensures that users can start a new test as soon as one is done or whenever the status is checked.

• Improved Error Handling Mechanism In case of an exception or an error, the Start button is immediately re-enabled. This allows users to start a new test after fixing errors without having to restart the application.

These enhancements aim to enforce better user interface management, limiting potential mishaps caused by accidental multiple clicks, as well as ensuring a smoother user experience during the testing process.

[ghost]

👇 Click on the image for a new way to code review

#### [](https://app.codesee.io/r/reviews?pr=1467&src=https%3A%2F%2Fgithub.com%2Ftransmission-remote-gui%2Ftransgui) #### Legend

[fredo-47]

I can confirm that transgui is broken when used with newer transmission-daemon dev-versions, regarding functions "Move torrent data" and "Delete torrent data".

After I built transgui with the file changes suggested above, transgui is able to perform these functions again as expected.

[PeterDaveHello]

@fredo-47 Thanks for the contribution. I will take my spare time to review them as soon as possible.

[killemov]

A bot wrote:

It's generally a good practice to add comments explaining the purpose of the code, especially when making changes. Consider adding a comment near this code block explaining the purpose of the condition and what it checks for.

Too bad the bot does not warn against over-commenting, so much comments that important stuff gets lost or comments that state the obvious. Only if requesting the field from the request argument had some unforeseen side-effect, a comment about that would be valid otherwise the code itself is clear enough.

[killemov]

Can anyone tell me if the used JSON parser can handle truthy and falsy values correctly?

[lighterowl]

Can anyone tell me if the used JSON parser can handle truthy and falsy values correctly?

It's not that simple to determine because you need to explicitly state the type you're accessing when fetching values from a parsed object. However, when you access Booleans, it seems to kind-of-sort-of have the "truthy-falsy" behaviour. Have a look yourself :

unit TestCase1;

{$mode objfpc}{$H+}

interface

uses
  Classes, SysUtils, fpcunit, testregistry, fpjson, jsonparser;

type

  JsonTruthyFalsy= class(TTestCase)
  published
    procedure BoolAsInteger;
    procedure StringboolAsInteger;
    procedure StringboolAsBool;
    procedure StringAsBool;
    procedure IntegerAsBool;
  end;

implementation

procedure JsonTruthyFalsy.BoolAsInteger;
var
  obj : TJSONObject;
begin
  obj := GetJSON('{"foobar":true}') as TJSONObject;
  AssertEquals(obj.Integers['foobar'], 1);
end;

procedure JsonTruthyFalsy.StringboolAsInteger;
var
  obj : TJSONObject;
begin
  obj := GetJSON('{"foobar":"true"}') as TJSONObject;
  AssertEquals(obj.Integers['foobar'], 1);
end;

procedure JsonTruthyFalsy.StringboolAsBool;
var
  obj : TJSONObject;
begin
  obj := GetJSON('{"foobar":"true"}') as TJSONObject;
  AssertTrue(obj.Booleans['foobar']);
end;

procedure JsonTruthyFalsy.StringAsBool;
var
  obj : TJSONObject;
begin
  obj := GetJSON('{"foobar":"transgui"}') as TJSONObject;
  AssertTrue(obj.Booleans['foobar']);
end;

procedure JsonTruthyFalsy.IntegerAsBool;
var
  obj : TJSONObject;
begin
  obj := GetJSON('{"foobar":1}') as TJSONObject;
  AssertTrue(obj.Booleans['foobar']);
end;

initialization
  RegisterTest(JsonTruthyFalsy);
end.

Out of those, StringboolAsInteger and StringAsBool fail.

This means that "true" and 1 are converted to true when using .Booleans.

[killemov]

Good, this means the original Integer "getter" and comparison to 0 were actually bad programming and that it's fixed even if the actual values in the JSON are 0 or 1.