fix(router): metadata field update in merchant_account and merchant_connector_account (#359)

Co-authored-by: Abhishek Marrivagu <abhi.codes10@gmail.com>

crates/router/src/core/admin.rs

@@ -1,3 +1,4 @@
+use common_utils::ext_traits::ValueExt;
 use error_stack::{report, FutureExt, ResultExt};
 use uuid::Uuid;
 
@@ -12,7 +13,7 @@ use crate::{
         storage::{self, MerchantAccount},
         transformers::{ForeignInto, ForeignTryInto},
     },
-    utils::{self, OptionExt, ValueExt},
+    utils::{self, OptionExt},
 };
 
 #[inline]
@@ -29,21 +30,23 @@ pub async fn create_merchant_account(
     db: &dyn StorageInterface,
     req: api::CreateMerchantAccount,
 ) -> RouterResponse<api::MerchantAccountResponse> {
-    let publishable_key = &format!("pk_{}", create_merchant_api_key());
-    let api_key = create_merchant_api_key();
-    let mut response = req.clone();
-    response.api_key = Some(api_key.to_owned().into());
-    response.publishable_key = Some(publishable_key.to_owned());
-    let merchant_details =
+    let publishable_key = Some(format!("pk_{}", create_merchant_api_key()));
+
+    let api_key = Some(create_merchant_api_key().into());
+
+    let merchant_details = Some(
         utils::Encode::<api::MerchantDetails>::encode_to_value(&req.merchant_details)
             .change_context(errors::ApiErrorResponse::InvalidDataValue {
                 field_name: "merchant_details",
-            })?;
-    let webhook_details =
+            })?,
+    );
+
+    let webhook_details = Some(
         utils::Encode::<api::WebhookDetails>::encode_to_value(&req.webhook_details)
             .change_context(errors::ApiErrorResponse::InvalidDataValue {
                 field_name: "webhook details",
-            })?;
+            })?,
+    );
 
     if let Some(ref routing_algorithm) = req.routing_algorithm {
         let _: api::RoutingAlgorithm = routing_algorithm
@@ -58,31 +61,36 @@ pub async fn create_merchant_account(
     let merchant_account = storage::MerchantAccountNew {
         merchant_id: req.merchant_id,
         merchant_name: req.merchant_name,
-        api_key: Some(api_key.to_string().into()),
-        merchant_details: Some(merchant_details),
+        api_key,
+        merchant_details,
         return_url: req.return_url,
-        webhook_details: Some(webhook_details),
+        webhook_details,
         routing_algorithm: req.routing_algorithm,
         sub_merchants_enabled: req.sub_merchants_enabled,
         parent_merchant_id: get_parent_merchant(
             db,
-            &req.sub_merchants_enabled,
+            req.sub_merchants_enabled,
             req.parent_merchant_id,
         )
         .await?,
         enable_payment_response_hash: req.enable_payment_response_hash,
         payment_response_hash_key: req.payment_response_hash_key,
         redirect_to_merchant_with_http_post: req.redirect_to_merchant_with_http_post,
-        publishable_key: Some(publishable_key.to_owned()),
+        publishable_key,
         locker_id: req.locker_id,
+        metadata: req.metadata,
     };
 
-    db.insert_merchant(merchant_account)
+    let merchant_account = db
+        .insert_merchant(merchant_account)
         .await
         .map_err(|error| {
             error.to_duplicate_response(errors::ApiErrorResponse::DuplicateMerchantAccount)
         })?;
-    Ok(service_api::ApplicationResponse::Json(response))
+
+    Ok(service_api::ApplicationResponse::Json(
+        merchant_account.foreign_into(),
+    ))
 }
 
 pub async fn get_merchant_account(
@@ -95,34 +103,10 @@ pub async fn get_merchant_account(
         .map_err(|error| {
             error.to_not_found_response(errors::ApiErrorResponse::MerchantAccountNotFound)
         })?;
-    let merchant_details = merchant_account
-        .merchant_details
-        .parse_value("MerchantDetails")
-        .change_context(errors::ApiErrorResponse::InternalServerError)?;
-    let webhook_details = merchant_account
-        .webhook_details
-        .parse_value("WebhookDetails")
-        .change_context(errors::ApiErrorResponse::InternalServerError)?;
-    let response = api::MerchantAccountResponse {
-        merchant_id: req.merchant_id,
-        merchant_name: merchant_account.merchant_name,
-        api_key: merchant_account.api_key,
-        merchant_details,
-        return_url: merchant_account.return_url,
-        webhook_details,
-        routing_algorithm: merchant_account.routing_algorithm,
-        sub_merchants_enabled: merchant_account.sub_merchants_enabled,
-        parent_merchant_id: merchant_account.parent_merchant_id,
-        enable_payment_response_hash: Some(merchant_account.enable_payment_response_hash),
-        payment_response_hash_key: merchant_account.payment_response_hash_key,
-        redirect_to_merchant_with_http_post: Some(
-            merchant_account.redirect_to_merchant_with_http_post,
-        ),
-        metadata: None,
-        publishable_key: merchant_account.publishable_key,
-        locker_id: merchant_account.locker_id,
-    };
-    Ok(service_api::ApplicationResponse::Json(response))
+
+    Ok(service_api::ApplicationResponse::Json(
+        merchant_account.foreign_into(),
+    ))
 }
 
 pub async fn merchant_account_update(
@@ -159,76 +143,56 @@ pub async fn merchant_account_update(
             .attach_printable("Invalid routing algorithm given")?;
     }
 
-    let mut response = req.clone();
-
-    let encode_error_handler =
-        |value: &str| format!("Unable to encode to serde_json::Value, {value}");
-
     let updated_merchant_account = storage::MerchantAccountUpdate::Update {
-        merchant_id: merchant_id.to_string(),
-        merchant_name: req
-            .merchant_name
-            .or_else(|| merchant_account.merchant_name.to_owned()),
-        api_key: merchant_account.api_key.clone(),
-        merchant_details: if req.merchant_details.is_some() {
-            Some(
-                utils::Encode::<api::MerchantDetails>::encode_to_value(&req.merchant_details)
-                    .change_context(errors::ApiErrorResponse::InternalServerError)
-                    .attach_printable_lazy(|| encode_error_handler("MerchantDetails"))?,
-            )
-        } else {
-            merchant_account.merchant_details.to_owned()
-        },
-        return_url: req
-            .return_url
-            .or_else(|| merchant_account.return_url.to_owned()),
-        webhook_details: if req.webhook_details.is_some() {
-            Some(
-                utils::Encode::<api::WebhookDetails>::encode_to_value(&req.webhook_details)
-                    .change_context(errors::ApiErrorResponse::InternalServerError)
-                    .attach_printable_lazy(|| encode_error_handler("WebhookDetails"))?,
-            )
-        } else {
-            merchant_account.webhook_details.to_owned()
-        },
-        routing_algorithm: req
-            .routing_algorithm
-            .or_else(|| merchant_account.routing_algorithm.clone()),
-        sub_merchants_enabled: req
-            .sub_merchants_enabled
-            .or(merchant_account.sub_merchants_enabled),
+        merchant_name: req.merchant_name,
+
+        merchant_details: req
+            .merchant_details
+            .as_ref()
+            .map(utils::Encode::<api::MerchantDetails>::encode_to_value)
+            .transpose()
+            .change_context(errors::ApiErrorResponse::InternalServerError)?,
+
+        return_url: req.return_url,
+
+        webhook_details: req
+            .webhook_details
+            .as_ref()
+            .map(utils::Encode::<api::WebhookDetails>::encode_to_value)
+            .transpose()
+            .change_context(errors::ApiErrorResponse::InternalServerError)?,
+
+        routing_algorithm: req.routing_algorithm,
+        sub_merchants_enabled: req.sub_merchants_enabled,
+
         parent_merchant_id: get_parent_merchant(
             db,
-            &req.sub_merchants_enabled
+            req.sub_merchants_enabled
                 .or(merchant_account.sub_merchants_enabled),
             req.parent_merchant_id
                 .or_else(|| merchant_account.parent_merchant_id.clone()),
         )
         .await?,
-        enable_payment_response_hash: req
-            .enable_payment_response_hash
-            .or(Some(merchant_account.enable_payment_response_hash)),
-        payment_response_hash_key: req
-            .payment_response_hash_key
-            .or_else(|| merchant_account.payment_response_hash_key.to_owned()),
-        redirect_to_merchant_with_http_post: req
-            .redirect_to_merchant_with_http_post
-            .or(Some(merchant_account.redirect_to_merchant_with_http_post)),
-        publishable_key: req
-            .publishable_key
-            .or_else(|| merchant_account.publishable_key.clone()),
-        locker_id: req
-            .locker_id
-            .or_else(|| merchant_account.locker_id.to_owned()),
+        enable_payment_response_hash: req.enable_payment_response_hash,
+        payment_response_hash_key: req.payment_response_hash_key,
+        redirect_to_merchant_with_http_post: req.redirect_to_merchant_with_http_post,
+        locker_id: req.locker_id,
+        metadata: req.metadata,
+        merchant_id: merchant_account.merchant_id.to_owned(),
+        api_key: None,
+        publishable_key: None,
     };
-    response.merchant_id = merchant_id.to_string();
-    response.api_key = merchant_account.api_key.to_owned();
 
-    db.update_merchant(merchant_account, updated_merchant_account)
+    let response = db
+        .update_merchant(merchant_account, updated_merchant_account)
         .await
-        .change_context(errors::ApiErrorResponse::InternalServerError)
-        .attach_printable_lazy(|| format!("Failed while updating merchant: {}", merchant_id))?;
-    Ok(service_api::ApplicationResponse::Json(response))
+        .map_err(|error| {
+            error.to_not_found_response(errors::ApiErrorResponse::MerchantAccountNotFound)
+        })?;
+
+    Ok(service_api::ApplicationResponse::Json(
+        response.foreign_into(),
+    ))
 }
 
 pub async fn merchant_account_delete(
@@ -250,7 +214,7 @@ pub async fn merchant_account_delete(
 
 async fn get_parent_merchant(
     db: &dyn StorageInterface,
-    sub_merchants_enabled: &Option<bool>,
+    sub_merchants_enabled: Option<bool>,
     parent_merchant: Option<String>,
 ) -> RouterResult<Option<String>> {
     Ok(match sub_merchants_enabled {
@@ -445,12 +409,11 @@ pub async fn update_payment_connector(
         connector_type: Some(req.connector_type.foreign_into()),
         connector_name: Some(req.connector_name),
         merchant_connector_id: Some(merchant_connector_id),
-        connector_account_details: req
-            .connector_account_details
-            .or_else(|| Some(Secret::new(mca.connector_account_details.to_owned()))),
+        connector_account_details: req.connector_account_details,
         payment_methods_enabled,
         test_mode: mca.test_mode,
         disabled: req.disabled.or(mca.disabled),
+        metadata: req.metadata,
     };
 
     let updated_mca = db
@@ -471,7 +434,7 @@ pub async fn update_payment_connector(
         test_mode: updated_mca.test_mode,
         disabled: updated_mca.disabled,
         payment_methods_enabled: req.payment_methods_enabled,
-        metadata: req.metadata,
+        metadata: updated_mca.metadata,
     };
     Ok(service_api::ApplicationResponse::Json(response))
 }

crates/router/src/db/merchant_account.rs

@@ -143,6 +143,7 @@ impl MerchantAccountInterface for MockDb {
             publishable_key: merchant_account.publishable_key,
             storage_scheme: enums::MerchantStorageScheme::PostgresOnly,
             locker_id: merchant_account.locker_id,
+            metadata: merchant_account.metadata,
         };
         accounts.push(account.clone());
         Ok(account)

crates/router/src/types/api/admin.rs

@@ -1,10 +1,36 @@
 pub use api_models::admin::{
-    CreateMerchantAccount, DeleteMcaResponse, DeleteResponse, MerchantConnectorId, MerchantDetails,
-    MerchantId, PaymentConnectorCreate, PaymentMethods, RoutingAlgorithm, WebhookDetails,
+    CreateMerchantAccount, DeleteMcaResponse, DeleteResponse, MerchantAccountResponse,
+    MerchantConnectorId, MerchantDetails, MerchantId, PaymentConnectorCreate, PaymentMethods,
+    RoutingAlgorithm, WebhookDetails,
 };
 
+use crate::types::{storage, transformers::Foreign};
+
+impl From<Foreign<storage::MerchantAccount>> for Foreign<MerchantAccountResponse> {
+    fn from(value: Foreign<storage::MerchantAccount>) -> Self {
+        let item = value.0;
+        MerchantAccountResponse {
+            merchant_id: item.merchant_id,
+            merchant_name: item.merchant_name,
+            api_key: item.api_key,
+            return_url: item.return_url,
+            enable_payment_response_hash: item.enable_payment_response_hash,
+            payment_response_hash_key: item.payment_response_hash_key,
+            redirect_to_merchant_with_http_post: item.redirect_to_merchant_with_http_post,
+            merchant_details: item.merchant_details,
+            webhook_details: item.webhook_details,
+            routing_algorithm: item.routing_algorithm,
+            sub_merchants_enabled: item.sub_merchants_enabled,
+            parent_merchant_id: item.parent_merchant_id,
+            publishable_key: item.publishable_key,
+            metadata: item.metadata,
+            locker_id: item.locker_id,
+        }
+        .into()
+    }
+}
+
 //use serde::{Serialize, Deserialize};
-pub use self::CreateMerchantAccount as MerchantAccountResponse;
 
 //use crate::newtype;