From 63d29e52f3a663c3d1e0298e43f67c1aa1ad06f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lloren=C3=A7=20Muntaner?= <llorenc.muntaner@dfinity.org>
Date: Wed, 22 Nov 2023 20:30:10 +0100
Subject: [PATCH] Chore: Change "?" for "Option" for optional fields (#480)

# Motivation

Using `?` is not the same as `Option`. With `?` the field is optional,
but with `Option`, the field is mandatory, but the value can be
`undefined`.

This small distinction is important when we render the information of
proposals in NNS Dapp. We convert the actions to camelcase and remove
`[] | [T]`.

If we use `?`, we might forget to convert it and it will never show up
in NNS Dapp. By using `Option`, we make sure that the field will always
we present in the converters.

You can see the example in the `params`, `maxDirectParticipationIcpE8s`
and `minDirectParticipationIcpE8s` fields for the (deprecated)
`OpenSnsTokenSwap` proposal action that I had to change when I moved
from `?` to `Option`.

# Changes

* Change the `?` in `types/governance_converters.ts` for `Option`. Only
for the fields related to proposals.

# Tests

* Fix a converter after changes.

# Todos

- [x] Add entry to changelog (if necessary).
---
 CHANGELOG.md                                  |   2 +
 .../governance/response.converters.ts         |  34 +++---
 .../nns/src/types/governance_converters.ts    | 112 +++++++++---------
 3 files changed, 77 insertions(+), 71 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b41761f83..41841f1c3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,8 @@
 
 ## Features
 
+- Substitute `?` fields with `Option` fields in the converters related to NNS proposals.
+
 # Release.2023.11.21-1400Z
 
 ## Overview
diff --git a/packages/nns/src/canisters/governance/response.converters.ts b/packages/nns/src/canisters/governance/response.converters.ts
index 20a4a6d80..a1bc6cc76 100644
--- a/packages/nns/src/canisters/governance/response.converters.ts
+++ b/packages/nns/src/canisters/governance/response.converters.ts
@@ -460,21 +460,25 @@ const toAction = (action: RawAction): Action => {
         targetSwapCanisterId: fromNullable(
           OpenSnsTokenSwap.target_swap_canister_id,
         ),
-        ...(params !== undefined && {
-          params: {
-            minParticipantIcpE8s: params.min_participant_icp_e8s,
-            maxIcpE8s: params.max_icp_e8s,
-            swapDueTimestampSeconds: params.swap_due_timestamp_seconds,
-            minParticipants: params.min_participants,
-            snsTokenE8s: params.sns_token_e8s,
-            maxParticipantIcpE8s: params.max_participant_icp_e8s,
-            minIcpE8s: params.min_icp_e8s,
-            saleDelaySeconds: fromNullable(params.sale_delay_seconds),
-            neuronBasketConstructionParameters: fromNullable(
-              params.neuron_basket_construction_parameters,
-            ),
-          },
-        }),
+        params: params && {
+          minParticipantIcpE8s: params.min_participant_icp_e8s,
+          maxIcpE8s: params.max_icp_e8s,
+          swapDueTimestampSeconds: params.swap_due_timestamp_seconds,
+          minParticipants: params.min_participants,
+          snsTokenE8s: params.sns_token_e8s,
+          maxParticipantIcpE8s: params.max_participant_icp_e8s,
+          minIcpE8s: params.min_icp_e8s,
+          saleDelaySeconds: fromNullable(params.sale_delay_seconds),
+          neuronBasketConstructionParameters: fromNullable(
+            params.neuron_basket_construction_parameters,
+          ),
+          maxDirectParticipationIcpE8s: fromNullable(
+            params.max_direct_participation_icp_e8s,
+          ),
+          minDirectParticipationIcpE8s: fromNullable(
+            params.min_direct_participation_icp_e8s,
+          ),
+        },
       },
     };
   }
diff --git a/packages/nns/src/types/governance_converters.ts b/packages/nns/src/types/governance_converters.ts
index 2dd6dd935..05756604f 100644
--- a/packages/nns/src/types/governance_converters.ts
+++ b/packages/nns/src/types/governance_converters.ts
@@ -190,7 +190,7 @@ export interface MergeRequest {
   targetNeuronId: NeuronId;
 }
 export interface StakeMaturity {
-  percentageToStake?: number;
+  percentageToStake: Option<number>;
 }
 export interface MergeMaturity {
   percentageToMerge: number;
@@ -217,9 +217,9 @@ export interface Motion {
   motionText: string;
 }
 export interface OpenSnsTokenSwap {
-  communityFundInvestmentE8s?: bigint;
-  targetSwapCanisterId?: Principal;
-  params?: {
+  communityFundInvestmentE8s: Option<bigint>;
+  targetSwapCanisterId: Option<Principal>;
+  params: Option<{
     minParticipantIcpE8s: bigint;
     maxIcpE8s: bigint;
     swapDueTimestampSeconds: bigint;
@@ -227,24 +227,24 @@ export interface OpenSnsTokenSwap {
     snsTokenE8s: bigint;
     maxParticipantIcpE8s: bigint;
     minIcpE8s: bigint;
-    saleDelaySeconds?: bigint;
-    neuronBasketConstructionParameters?: {
+    saleDelaySeconds: Option<bigint>;
+    neuronBasketConstructionParameters: Option<{
       // Keep snake case to avoid having to convert back and forth.
       dissolve_delay_interval_seconds: bigint;
       count: bigint;
-    };
-    maxDirectParticipationIcpE8s?: bigint;
-    minDirectParticipationIcpE8s?: bigint;
-  };
+    }>;
+    maxDirectParticipationIcpE8s: Option<bigint>;
+    minDirectParticipationIcpE8s: Option<bigint>;
+  }>;
 }
 export interface SetSnsTokenSwapOpenTimeWindow {
-  request?: {
-    openTimeWindow?: {
+  request: Option<{
+    openTimeWindow: Option<{
       startTimestampSeconds: bigint;
       endTimestampSeconds: bigint;
-    };
-  };
-  swapCanisterId?: string;
+    }>;
+  }>;
+  swapCanisterId: Option<string>;
 }
 export interface NetworkEconomics {
   neuronMinimumStake: E8s;
@@ -503,15 +503,15 @@ export interface ListNodeProvidersResponse {
 }
 
 export interface Percentage {
-  basisPoints?: bigint;
+  basisPoints: Option<bigint>;
 }
 
 export interface Duration {
-  seconds?: bigint;
+  seconds: Option<bigint>;
 }
 
 export interface GlobalTimeOfDay {
-  secondsAfterUtcMidnight?: bigint;
+  secondsAfterUtcMidnight: Option<bigint>;
 }
 
 export interface Countries {
@@ -519,42 +519,42 @@ export interface Countries {
 }
 
 export interface Tokens {
-  e8s?: bigint;
+  e8s: Option<bigint>;
 }
 
 export interface Image {
-  base64Encoding?: string;
+  base64Encoding: Option<string>;
 }
 
 export interface LedgerParameters {
-  transactionFee?: Tokens;
-  tokenSymbol?: string;
-  tokenLogo?: Image;
-  tokenName?: string;
+  transactionFee: Option<Tokens>;
+  tokenSymbol: Option<string>;
+  tokenLogo: Option<Image>;
+  tokenName: Option<string>;
 }
 
 export interface VotingRewardParameters {
-  rewardRateTransitionDuration?: Duration;
-  initialRewardRate?: Percentage;
-  finalRewardRate?: Percentage;
+  rewardRateTransitionDuration: Option<Duration>;
+  initialRewardRate: Option<Percentage>;
+  finalRewardRate: Option<Percentage>;
 }
 
 export interface GovernanceParameters {
-  neuronMaximumDissolveDelayBonus?: Percentage;
-  neuronMaximumAgeForAgeBonus?: Duration;
-  neuronMaximumDissolveDelay?: Duration;
-  neuronMinimumDissolveDelayToVote?: Duration;
-  neuronMaximumAgeBonus?: Percentage;
-  neuronMinimumStake?: Tokens;
-  proposalWaitForQuietDeadlineIncrease?: Duration;
-  proposalInitialVotingPeriod?: Duration;
-  proposalRejectionFee?: Tokens;
-  votingRewardParameters?: VotingRewardParameters;
+  neuronMaximumDissolveDelayBonus: Option<Percentage>;
+  neuronMaximumAgeForAgeBonus: Option<Duration>;
+  neuronMaximumDissolveDelay: Option<Duration>;
+  neuronMinimumDissolveDelayToVote: Option<Duration>;
+  neuronMaximumAgeBonus: Option<Percentage>;
+  neuronMinimumStake: Option<Tokens>;
+  proposalWaitForQuietDeadlineIncrease: Option<Duration>;
+  proposalInitialVotingPeriod: Option<Duration>;
+  proposalRejectionFee: Option<Tokens>;
+  votingRewardParameters: Option<VotingRewardParameters>;
 }
 
 export interface NeuronBasketConstructionParameters {
-  dissolveDelayInterval?: Duration;
-  count?: bigint;
+  dissolveDelayInterval: Option<Duration>;
+  count: Option<bigint>;
 }
 export interface SwapParameters {
   minimumParticipants: Option<bigint>;
@@ -574,15 +574,15 @@ export interface SwapParameters {
 }
 
 export interface SwapDistribution {
-  total?: Tokens;
+  total: Option<Tokens>;
 }
 
 export interface NeuronDistribution {
-  controller?: PrincipalString;
-  dissolveDelay?: Duration;
-  memo?: bigint;
-  vestingPeriod?: Duration;
-  stake?: Tokens;
+  controller: Option<PrincipalString>;
+  dissolveDelay: Option<Duration>;
+  memo: Option<bigint>;
+  vestingPeriod: Option<Duration>;
+  stake: Option<Tokens>;
 }
 
 export interface DeveloperDistribution {
@@ -590,20 +590,20 @@ export interface DeveloperDistribution {
 }
 
 export interface InitialTokenDistribution {
-  treasuryDistribution?: SwapDistribution;
-  developerDistribution?: DeveloperDistribution;
-  swapDistribution?: SwapDistribution;
+  treasuryDistribution: Option<SwapDistribution>;
+  developerDistribution: Option<DeveloperDistribution>;
+  swapDistribution: Option<SwapDistribution>;
 }
 
 export interface CreateServiceNervousSystem {
-  url?: string;
-  governanceParameters?: GovernanceParameters;
+  url: Option<string>;
+  governanceParameters: Option<GovernanceParameters>;
   fallbackControllerPrincipalIds: Array<PrincipalString>;
-  logo?: Image;
-  name?: string;
-  ledgerParameters?: LedgerParameters;
-  description?: string;
+  logo: Option<Image>;
+  name: Option<string>;
+  ledgerParameters: Option<LedgerParameters>;
+  description: Option<string>;
   dappCanisters: Array<CanisterIdString>;
-  swapParameters?: SwapParameters;
-  initialTokenDistribution?: InitialTokenDistribution;
+  swapParameters: Option<SwapParameters>;
+  initialTokenDistribution: Option<InitialTokenDistribution>;
 }