fix(cargo-shuttle): fix --name priority over config id (#2082)

* wip: fix --name priority over config id

* fix: priority order

Author: jonaro00

cargo-shuttle/src/config.rs

@@ -38,6 +38,7 @@ impl ConfigManager for LocalConfigManager {
 #[derive(Deserialize, Serialize, Default)]
 pub struct ProjectConfig {
     // unused on new platform, but still used for project names in local runs
+    // TODO: remove and use the crate/workspace name instead
     pub name: Option<String>,
     /// Deprecated, now [`ProjectDeployConfig::include`]
     pub assets: Option<Vec<String>>,
@@ -125,7 +126,29 @@ impl RequestContext {
             // Command-line id parameter trumps everything
             (Some(id_from_args), _) => {
                 trace!("using command-line project id");
-                config.id = Some(id_from_args.clone());
+
+                // Validate format of explicitly given project id and change the ULID to uppercase if it is lowercase
+                let id_to_use = if let Some(proj_id_uppercase) =
+                    id_from_args.strip_prefix("proj_").and_then(|suffix| {
+                        // Soft (dumb) validation of ULID format (ULIDs are 26 chars)
+                        (suffix.len() == 26)
+                            .then_some(format!("proj_{}", suffix.to_ascii_uppercase()))
+                    }) {
+                    if *id_from_args != proj_id_uppercase {
+                        eprintln!("INFO: Converted project id to '{}'", proj_id_uppercase);
+
+                        proj_id_uppercase
+                    } else {
+                        id_from_args.clone()
+                    }
+                } else {
+                    // TODO: eprintln a warning?
+                    tracing::warn!("project id with bad format detected: '{id_from_args}'");
+
+                    id_from_args.clone()
+                };
+
+                config.id = Some(id_to_use);
             }
             // If key exists in config then keep it as it is
             (None, Some(_)) => {
@@ -147,9 +170,7 @@ impl RequestContext {
     }
 
     pub fn save_local_internal(&mut self) -> Result<()> {
-        // if self.project_internal.is_some() {
         self.project_internal.as_ref().unwrap().save()?;
-        // }
 
         // write updated gitignore file to root of workspace
         // TODO: assumes git is used
@@ -169,9 +190,7 @@ impl RequestContext {
 
     /// Load the Shuttle.toml project configuration at the given `working_directory`
     pub fn load_local_config(&mut self, project_args: &ProjectArgs) -> Result<()> {
-        let project = Self::get_local_config(project_args)?;
-
-        self.project = Some(project);
+        self.project = Some(Self::get_local_config(project_args)?);
 
         Ok(())
     }

cargo-shuttle/src/lib.rs

@@ -652,86 +652,66 @@ impl Shuttle {
         trace!("project arguments: {project_args:?}");
 
         self.ctx.load_local_config(project_args)?;
-        // load project id from args if given or from file if exists
+        // load project id from args if given or from internal config file if present
         self.ctx.load_local_internal_config(project_args)?;
 
-        if let Some(id) = project_args.id.as_ref() {
-            // Validate format of explicitly given project id and change the ULID to uppercase if it is lowercase
-            if let Some(proj_id_uppercase) = id.strip_prefix("proj_").and_then(|suffix| {
-                // Soft (dumb) validation of ULID format (ULIDs are 26 chars)
-                (suffix.len() == 26).then_some(format!("proj_{}", suffix.to_ascii_uppercase()))
-            }) {
-                if *id != proj_id_uppercase {
-                    eprintln!("INFO: Converted project id to '{}'", proj_id_uppercase);
-                    self.ctx.set_project_id(proj_id_uppercase);
-                }
-            } else {
-                // TODO: eprintln a warning?
-                warn!("project id with bad format detected: '{id}'");
-            }
-
-            // if linking, save config
-            if do_linking {
-                eprintln!("Linking to project {}", self.ctx.project_id());
-                self.ctx.save_local_internal()?;
-            }
-        }
-        if self.ctx.project_id_found() {
-            // --id takes prio over --name, and at this point we know the id, so we're done
-            return Ok(());
-        }
-
-        // translate project name to project id if a name was given
-        if let Some(name) = project_args.name.as_ref() {
-            let client = self.client.as_ref().unwrap();
-            trace!(%name, "looking up project id from project name");
-            if let Some(proj) = client
-                .get_projects_list()
-                .await?
-                .into_inner()
-                .projects
-                .into_iter()
-                .find(|p| p.name == *name)
-            {
-                trace!("found project by name");
-                self.ctx.set_project_id(proj.id);
-            } else {
-                trace!("did not find project by name");
-                if create_missing_project {
-                    trace!("creating project since it was not found");
-                    // This is a side effect (non-primary output), so OutputMode::Json is not considered
-                    let proj = client.create_project(name).await?.into_inner();
-                    eprintln!("Created project '{}' with id {}", proj.name, proj.id);
+        // If project id was not given via arg but a name was, try to translate the project name to a project id.
+        // (A --name from args takes precedence over an id from internal config.)
+        if project_args.id.is_none() {
+            if let Some(name) = project_args.name.as_ref() {
+                let client = self.client.as_ref().unwrap();
+                trace!(%name, "looking up project id from project name");
+                if let Some(proj) = client
+                    .get_projects_list()
+                    .await?
+                    .into_inner()
+                    .projects
+                    .into_iter()
+                    .find(|p| p.name == *name)
+                {
+                    trace!("found project by name");
                     self.ctx.set_project_id(proj.id);
+                } else {
+                    trace!("did not find project by name");
+                    if create_missing_project {
+                        trace!("creating project since it was not found");
+                        // This is a side effect (non-primary output), so OutputMode::Json is not considered
+                        let proj = client.create_project(name).await?.into_inner();
+                        eprintln!("Created project '{}' with id {}", proj.name, proj.id);
+                        self.ctx.set_project_id(proj.id);
+                    } else if do_linking {
+                        self.project_link_interactive().await?;
+                        return Ok(());
+                    } else {
+                        bail!(
+                            "Project with name '{}' not found in your project list. \
+                            Use 'shuttle project link' to create it or link an existing project.",
+                            name
+                        );
+                    }
                 }
             }
         }
 
         match (self.ctx.project_id_found(), do_linking) {
-            // if project id is known and we are linking, save config
             (true, true) => {
-                eprintln!("Linking to project {}", self.ctx.project_id());
-                self.ctx.save_local_internal()?;
+                let arg_given = project_args.id.is_some() || project_args.name.is_some();
+                if arg_given {
+                    // project id was found via explicitly given arg, save config
+                    eprintln!("Linking to project {}", self.ctx.project_id());
+                    self.ctx.save_local_internal()?;
+                } else {
+                    // project id was found but not given via arg, ask the user interactively
+                    self.project_link_interactive().await?;
+                }
             }
             // if project id is known, we are done and nothing more to do
             (true, false) => (),
-            // we still don't know the project id but want to link
-            (false, true) => {
+            // we still don't know the project id, so ask the user interactively
+            (false, _) => {
+                trace!("no project id found");
                 self.project_link_interactive().await?;
             }
-            // we still don't know the project id
-            (false, false) => {
-                // if a name was given but no project was found, (incorrectly) set the id to the name so that an api error will be encountered
-                // TODO: return an error here instead of letting an api error happen?
-                if let Some(name) = project_args.name.as_ref() {
-                    warn!("using project name as the id");
-                    self.ctx.set_project_id(name.clone());
-                } else {
-                    // otherwise, we have to do an explicit linking
-                    trace!("no project id found");
-                    self.project_link_interactive().await?;
-                }
-            }
         }
 
         Ok(())
@@ -1049,16 +1029,17 @@ impl Shuttle {
     }
 
     async fn deployments_list(&self, page: u32, limit: u32, table_args: TableArgs) -> Result<()> {
-        let client = self.client.as_ref().unwrap();
         if limit == 0 {
+            warn!("Limit is set to 0, no deployments will be listed.");
             return Ok(());
         }
-        let proj_name = self.ctx.project_name();
+        let client = self.client.as_ref().unwrap();
+        let pid = self.ctx.project_id();
 
         // fetch one additional to know if there is another page available
         let limit = limit + 1;
         let (deployments, raw_json) = client
-            .get_deployments(self.ctx.project_id(), page as i32, limit as i32)
+            .get_deployments(pid, page as i32, limit as i32)
             .await?
             .into_parts();
         let mut deployments = deployments.deployments;
@@ -1072,10 +1053,7 @@ impl Shuttle {
         match self.output_mode {
             OutputMode::Normal => {
                 let table = deployments_table(&deployments, table_args.raw);
-                println!(
-                    "{}",
-                    format!("Deployments in project '{}'", proj_name).bold()
-                );
+                println!("{}", format!("Deployments in project '{}'", pid).bold());
                 println!("{table}");
                 if page_hint {
                     println!("View the next page using `--page {}`", page + 1);