From 20fd9f4b9648c58569d5b3d03a0e26a84d93e4a1 Mon Sep 17 00:00:00 2001
From: Dmitry Bushev <bushevdv@gmail.com>
Date: Tue, 6 Dec 2022 20:43:33 +0300
Subject: [PATCH] Ensure project name starts with uppercase (#3947)

Fixes the regression when IDE fails to create a project from template. Project name should start with an upper case letter to pass the server side validation.
---
 .../double-representation/src/name/project.rs | 97 +++++++++++++++++++
 app/gui/src/controller/ide.rs                 |  3 +-
 app/gui/src/controller/ide/desktop.rs         | 23 +++--
 app/gui/src/presenter.rs                      | 29 +++---
 4 files changed, 133 insertions(+), 19 deletions(-)

diff --git a/app/gui/controller/double-representation/src/name/project.rs b/app/gui/controller/double-representation/src/name/project.rs
index ea3b006d21..c19e598285 100644
--- a/app/gui/controller/double-representation/src/name/project.rs
+++ b/app/gui/controller/double-representation/src/name/project.rs
@@ -28,6 +28,103 @@ pub const STANDARD_BASE_LIBRARY_PATH: &str = concatcp!(STANDARD_NAMESPACE, ".",
 
 
 
+// ================
+// === Template ===
+// ================
+
+#[allow(missing_docs)]
+#[derive(Copy, Clone, Debug, Fail)]
+pub enum InvalidTemplateName {
+    #[fail(display = "The template name contains invalid characters.")]
+    ContainsInvalidCharacters,
+}
+
+/// The project template name.
+#[derive(Clone, Debug)]
+pub struct Template {
+    name: String,
+}
+
+impl Template {
+    /// Create the project template from string.
+    ///
+    /// # Example
+    ///
+    /// ```rust
+    /// # use double_representation::name::project::Template;
+    /// assert!(Template::from_text("hello").is_ok());
+    /// assert!(Template::from_text("hello_world").is_err());
+    /// ```
+    pub fn from_text(text: impl AsRef<str>) -> FallibleResult<Self> {
+        if text.as_ref().contains(|c: char| !c.is_ascii_alphanumeric()) {
+            Err(InvalidTemplateName::ContainsInvalidCharacters.into())
+        } else {
+            Ok(Template { name: text.as_ref().to_owned() })
+        }
+    }
+
+    /// Create the project template from string without validation.
+    pub fn unsafe_from_text(text: impl AsRef<str>) -> Self {
+        Template { name: text.as_ref().to_owned() }
+    }
+
+    /// Create a project name from the template name.
+    /// # Example
+    ///
+    /// ```rust
+    /// # use double_representation::name::project::Template;
+    /// let template = Template::unsafe_from_text("hello");
+    /// assert_eq!(template.to_project_name(), "Hello".to_owned());
+    /// ```
+    pub fn to_project_name(&self) -> String {
+        let mut name = self.name.to_string();
+        // Capitalize
+        if let Some(r) = name.get_mut(0..1) {
+            r.make_ascii_uppercase();
+        }
+
+        name
+    }
+}
+
+// === Conversions From and Into String ===
+
+impl TryFrom<&str> for Template {
+    type Error = failure::Error;
+
+    fn try_from(text: &str) -> Result<Self, Self::Error> {
+        Self::from_text(text)
+    }
+}
+
+impl TryFrom<String> for Template {
+    type Error = failure::Error;
+
+    fn try_from(text: String) -> Result<Self, Self::Error> {
+        Self::from_text(text)
+    }
+}
+
+impl From<Template> for String {
+    fn from(template: Template) -> Self {
+        String::from(&template.name)
+    }
+}
+
+impl From<&Template> for String {
+    fn from(template: &Template) -> Self {
+        template.name.to_owned()
+    }
+}
+
+impl Display for Template {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.name)
+    }
+}
+
+
+
 // =====================
 // === QualifiedName ===
 // =====================
diff --git a/app/gui/src/controller/ide.rs b/app/gui/src/controller/ide.rs
index 976bfe199a..63a61971d4 100644
--- a/app/gui/src/controller/ide.rs
+++ b/app/gui/src/controller/ide.rs
@@ -7,6 +7,7 @@ use crate::prelude::*;
 
 use crate::notification;
 
+use double_representation::name::project;
 use mockall::automock;
 use parser_scala::Parser;
 
@@ -134,7 +135,7 @@ pub trait ManagingProjectAPI {
     ///
     /// `template` is an optional project template name. Available template names are defined in
     /// `lib/scala/pkg/src/main/scala/org/enso/pkg/Template.scala`.
-    fn create_new_project(&self, template: Option<String>) -> BoxFuture<FallibleResult>;
+    fn create_new_project(&self, template: Option<project::Template>) -> BoxFuture<FallibleResult>;
 
     /// Return a list of existing projects.
     fn list_projects(&self) -> BoxFuture<FallibleResult<Vec<ProjectMetadata>>>;
diff --git a/app/gui/src/controller/ide/desktop.rs b/app/gui/src/controller/ide/desktop.rs
index 660bc994ef..9cddb49bcf 100644
--- a/app/gui/src/controller/ide/desktop.rs
+++ b/app/gui/src/controller/ide/desktop.rs
@@ -11,6 +11,7 @@ use crate::controller::ide::API;
 use crate::ide::initializer;
 use crate::notification;
 
+use double_representation::name::project;
 use engine_protocol::project_manager;
 use engine_protocol::project_manager::MissingComponentAction;
 use engine_protocol::project_manager::ProjectMetadata;
@@ -119,22 +120,24 @@ impl API for Handle {
 
 impl ManagingProjectAPI for Handle {
     #[profile(Objective)]
-    fn create_new_project(&self, template: Option<String>) -> BoxFuture<FallibleResult> {
+    fn create_new_project(&self, template: Option<project::Template>) -> BoxFuture<FallibleResult> {
         async move {
             use model::project::Synchronized as Project;
 
             let list = self.project_manager.list_projects(&None).await?;
             let existing_names: HashSet<_> =
                 list.projects.into_iter().map(|p| p.name.into()).collect();
-            let name = template.clone().unwrap_or_else(|| UNNAMED_PROJECT_NAME.to_owned());
-            let name = choose_new_project_name(&existing_names, &name);
+            let name = make_project_name(&template);
+            let name = choose_unique_project_name(&existing_names, &name);
             let name = ProjectName::new_unchecked(name);
             let version =
                 enso_config::ARGS.preferred_engine_version.as_ref().map(ToString::to_string);
             let action = MissingComponentAction::Install;
 
-            let create_result =
-                self.project_manager.create_project(&name, &template, &version, &action).await?;
+            let create_result = self
+                .project_manager
+                .create_project(&name, &template.map(|t| t.into()), &version, &action)
+                .await?;
             let new_project_id = create_result.project_id;
             let project_mgr = self.project_manager.clone_ref();
             let new_project = Project::new_opened(&self.logger, project_mgr, new_project_id);
@@ -167,7 +170,7 @@ impl ManagingProjectAPI for Handle {
 
 /// Select a new name for the project in a form of <suggested_name>_N, where N is a unique sequence
 /// number.
-fn choose_new_project_name(existing_names: &HashSet<String>, suggested_name: &str) -> String {
+fn choose_unique_project_name(existing_names: &HashSet<String>, suggested_name: &str) -> String {
     let first_candidate = suggested_name.to_owned();
     let nth_project_name = |i| iformat!("{suggested_name}_{i}");
     let candidates = (1..).map(nth_project_name);
@@ -175,3 +178,11 @@ fn choose_new_project_name(existing_names: &HashSet<String>, suggested_name: &st
     // The iterator have no end, so we can safely unwrap.
     candidates.find(|c| !existing_names.contains(c)).unwrap()
 }
+
+/// Come up with a project name.
+fn make_project_name(template: &Option<project::Template>) -> String {
+    template
+        .as_ref()
+        .map(|t| t.to_project_name())
+        .unwrap_or_else(|| UNNAMED_PROJECT_NAME.to_owned())
+}
diff --git a/app/gui/src/presenter.rs b/app/gui/src/presenter.rs
index c44132fc98..5443c5946a 100644
--- a/app/gui/src/presenter.rs
+++ b/app/gui/src/presenter.rs
@@ -103,20 +103,25 @@ impl Model {
     #[profile(Task)]
     fn create_project(&self, template: Option<&str>) {
         let controller = self.controller.clone_ref();
-        let template = template.map(ToOwned::to_owned);
-        crate::executor::global::spawn(async move {
-            if let Ok(managing_api) = controller.manage_projects() {
-                if let Err(err) = managing_api.create_new_project(template.clone()).await {
-                    if let Some(template) = template {
-                        error!("Could not create new project from template {template}: {err}.");
-                    } else {
-                        error!("Could not create new project: {err}.");
+        if let Ok(template) =
+            template.map(double_representation::name::project::Template::from_text).transpose()
+        {
+            crate::executor::global::spawn(async move {
+                if let Ok(managing_api) = controller.manage_projects() {
+                    if let Err(err) = managing_api.create_new_project(template.clone()).await {
+                        if let Some(template) = template {
+                            error!("Could not create new project from template {template}: {err}.");
+                        } else {
+                            error!("Could not create new project: {err}.");
+                        }
                     }
+                } else {
+                    warn!("Project creation failed: no ProjectManagingAPI available.");
                 }
-            } else {
-                warn!("Project creation failed: no ProjectManagingAPI available.");
-            }
-        });
+            })
+        } else if let Some(template) = template {
+            error!("Invalid project template name: {template}");
+        };
     }
 }