Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 'account delete' command #2539

Merged
merged 19 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Short option for `--accounts-file` flag has been removed.
- Short option for `--contract-address` is now `-d` instead of `-a`.

#### Fixed
- `account delete` command. Now it isn't required to pass `--url` each time. It is needed to provide neither `--url` or `--network` argument only.

kkawula marked this conversation as resolved.
Show resolved Hide resolved
## [0.31.0] - 2024-09-26

### Cast
Expand Down
8 changes: 2 additions & 6 deletions crates/sncast/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,12 +430,8 @@ async fn run_async_command(
}

account::Commands::Delete(delete) => {
let provider = delete.rpc.get_provider(&config).await?;

let network_name = match delete.network {
Some(network) => network,
None => chain_id_to_network_name(get_chain_id(&provider).await?),
};
let network_name =
starknet_commands::account::delete::get_network_name(&delete, &config).await?;

let result = starknet_commands::account::delete::delete(
&delete.name,
Expand Down
22 changes: 20 additions & 2 deletions crates/sncast/src/starknet_commands/account/delete.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
use anyhow::{anyhow, bail, Context, Result};
use camino::Utf8PathBuf;
use clap::Args;
use clap::{ArgGroup, Args};
use promptly::prompt;
use serde_json::Map;
use sncast::helpers::configuration::CastConfig;
use sncast::helpers::rpc::RpcArgs;
use sncast::response::structs::AccountDeleteResponse;
use sncast::{chain_id_to_network_name, get_chain_id};

#[derive(Args, Debug)]
#[command(about = "Delete account information from the accounts file")]
#[command(group(ArgGroup::new("networks")
.args(&["url", "network"])
.required(true)
.multiple(false)))]
pub struct Delete {
/// Name of the account to be deleted
#[clap(short, long)]
Expand All @@ -22,7 +28,7 @@ pub struct Delete {
pub yes: bool,

#[clap(flatten)]
pub rpc: RpcArgs,
pub rpc: Option<RpcArgs>,
}

#[allow(clippy::too_many_arguments)]
Expand Down Expand Up @@ -71,3 +77,15 @@ pub fn delete(
let result = "Account successfully removed".to_string();
Ok(AccountDeleteResponse { result })
}

pub async fn get_network_name(delete: &Delete, config: &CastConfig) -> Result<String> {
kkawula marked this conversation as resolved.
Show resolved Hide resolved
match (&delete.rpc, &delete.network) {
(Some(rpc), None) => {
let provider = rpc.get_provider(config).await?;
let network_name = chain_id_to_network_name(get_chain_id(&provider).await?);
Ok(network_name)
}
(None, Some(network)) => Ok(network.clone()),
_ => unreachable!("Checked on clap level"),
}
}
96 changes: 88 additions & 8 deletions crates/sncast/tests/e2e/account/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ pub fn test_no_accounts_in_network() {
ACCOUNT_FILE_PATH,
"account",
"delete",
"--url",
URL,
"--name",
"user99",
"--network",
Expand All @@ -31,6 +29,31 @@ pub fn test_no_accounts_in_network() {
);
}

#[test]
pub fn test_no_accounts_in_network_url() {
let args = vec![
kkawula marked this conversation as resolved.
Show resolved Hide resolved
"--accounts-file",
ACCOUNT_FILE_PATH,
"account",
"delete",
"--url",
URL,
"--name",
"user99",
];

let snapbox = runner(&args);
let output = snapbox.assert().success();

assert_stderr_contains(
output,
indoc! {r"
command: account delete
error: Account with name user99 does not exist
"},
);
}

#[test]
pub fn test_account_does_not_exist() {
let args = vec![
Expand Down Expand Up @@ -68,8 +91,6 @@ pub fn test_delete_abort() {
&accounts_file_name,
"account",
"delete",
"--url",
URL,
"--name",
"user3",
"--network",
Expand Down Expand Up @@ -101,8 +122,6 @@ pub fn test_happy_case() {
&accounts_file_name,
"account",
"delete",
"--url",
URL,
"--name",
"user3",
"--network",
Expand All @@ -119,7 +138,7 @@ pub fn test_happy_case() {
}

#[test]
pub fn test_happy_case_without_network_args() {
pub fn test_happy_case_url() {
franciszekjob marked this conversation as resolved.
Show resolved Hide resolved
// Creating dummy accounts test file
let accounts_file_name = "temp_accounts.json";
let temp_dir = create_tempdir_with_accounts_file(accounts_file_name, true);
Expand All @@ -146,7 +165,7 @@ pub fn test_happy_case_without_network_args() {
}

#[test]
pub fn test_happy_case_with_yes_flag() {
pub fn test_happy_case_without_network_args() {
// Creating dummy accounts test file
let accounts_file_name = "temp_accounts.json";
let temp_dir = create_tempdir_with_accounts_file(accounts_file_name, true);
Expand All @@ -160,6 +179,31 @@ pub fn test_happy_case_with_yes_flag() {
"--url",
URL,
"--name",
"user0",
];

// Run test with an affirmative user input
let snapbox = runner(&args).current_dir(temp_dir.path()).stdin("Y");

snapbox.assert().success().stdout_matches(indoc! {r"
command: account delete
result: Account successfully removed
"});
}

#[test]
pub fn test_happy_case_with_yes_flag() {
// Creating dummy accounts test file
let accounts_file_name = "temp_accounts.json";
let temp_dir = create_tempdir_with_accounts_file(accounts_file_name, true);

// Now delete dummy account
let args = vec![
"--accounts-file",
&accounts_file_name,
"account",
"delete",
"--name",
"user3",
"--network",
"custom-network",
Expand All @@ -176,3 +220,39 @@ pub fn test_happy_case_with_yes_flag() {
result: Account successfully removed
"});
}

#[test]
pub fn test_conflicting_arguments() {
franciszekjob marked this conversation as resolved.
Show resolved Hide resolved
// Creating dummy accounts test file
let accounts_file_name = "temp_accounts.json";
let temp_dir = create_tempdir_with_accounts_file(accounts_file_name, true);

// Now delete dummy account
let args = vec![
"--accounts-file",
&accounts_file_name,
"account",
"delete",
"--url",
URL,
"--name",
"user3",
"--network",
"custom-network",
];

// Run test with an affirmative user input
let snapbox = runner(&args).current_dir(temp_dir.path()).stdin("Y");

let output = snapbox.assert().failure();
assert_stderr_contains(
output,
indoc! {r"
error: the argument '--url <URL>' cannot be used with '--network <NETWORK>'

Usage: sncast account delete --name <NAME> <--url <URL>|--network <NETWORK>>

For more information, try '--help'.
"},
);
}
franciszekjob marked this conversation as resolved.
Show resolved Hide resolved
Loading