Repository: juspay/hyperswitch
File: crates/router/src/core/payment_methods.rs (Rust)

Code before:
            None,
            None,
            None,
            None,
        )
        .await
        .attach_printable("Unable to create Payment method data")?;

        let updated_payment_method = db
            .update_payment_method(
                self.platform.get_provider().get_key_store(),
                self.payment_method.clone(),
                pm_update,
                self.platform.get_provider().get_account().storage_scheme,
            )
            .await
            .change_context(errors::ApiErrorResponse::InternalServerError)
            .attach_printable("Failed to update payment method in db")?;

        self.payment_method = updated_payment_method;
        Ok(())
    }

    pub fn generate_response(
        &self,
        card_cvc_token_details: Option<payment_methods::CardCVCTokenStorageDetails>,
    ) -> RouterResult<payment_methods::PaymentMethodResponse> {
        let response = pm_transforms::generate_payment_method_response(
            &self.payment_method,
            &None,
            Some(common_enums::StorageType::Persistent),
            card_cvc_token_details,
            self.payment_method.customer_id.clone(),
            None,
        )?;

        Ok(response)
    }
}


Review comment (suggestion):
Current code logic
```
pub fn is_payment_method_update_required(&self) -> bool {
        self.is_payment_method_metadata_update()
            || self.connector_token_details.is_some()
            || self.network_transaction_id.is_some()
    }
```

Currently, whenever a new parameter is added to the update request, this validation requires it to be included. Additionally, the validation is performed after the vaulting step, which is not ideal.
It would be better to introduce a dedicated validateUpdateRequest method and invoke it at the start of the function.

Code after:
                    self.platform,
                    self.profile,
                    current_vault_id,
                    fingerprint_id_from_vault,
                    &self
                        .payment_method
                        .customer_id
                        .to_owned()
                        .get_required_value("GlobalCustomerId")?,
                )
                .await
                .attach_printable("Failed to add payment method in vault")?;

                Ok((Some(vault_request_data), Some(vault_response)))
            }
        }
    }

    pub async fn update_payment_method_if_required(
        &mut self,
        vault_request_data: Option<domain::PaymentMethodVaultingData>,
        vault_resp: Option<pm_types::AddVaultResponse>,
    ) -> RouterResult<()> {
        if !self.request.is_payment_method_update_required() {
            return Ok(());
        }

        let db = self.state.store.as_ref();

        let pm_update = create_pm_additional_data_update(
            vault_request_data.as_ref(),
            self.state,
            self.platform.get_provider().get_key_store(),
            vault_resp
                .as_ref()
                .map(|resp| resp.vault_id.get_string_repr().to_owned()),
            vault_resp
                .as_ref()
                .and_then(|resp| resp.fingerprint_id.clone()),
            &self.payment_method,
            self.request.connector_token_details.clone(),
            self.request.network_transaction_id.clone(),
            None,
            None,
            None,
            None,
            None,
        )
        .await
        .attach_printable("Unable to create Payment method data")?;