feat: adding workarounds if post creation fails. (#1109)

rs/cli/src/commands/mod.rs

@@ -138,6 +138,11 @@ pub struct DiscourseOpts {
     /// Api url used to interact with the forum
     #[clap(long, env = "DISCOURSE_API_URL")]
     pub(crate) discourse_api_url: Option<String>,
+
+    /// Skip forum post creation all together, also will not
+    /// prompt user for the link
+    #[clap(long, env = "DISCOURSE_SKIP_POST_CREATION")]
+    pub(crate) discourse_skip_post_creation: bool,
 }
 
 #[derive(Parser, Debug)]

rs/cli/src/commands/subnet/replace.rs

@@ -80,25 +80,28 @@ impl ExecutableCommand for Replace {
         if let Some(runner_proposal) = runner.propose_subnet_change(subnet_change_response, ctx.forum_post_link()).await? {
             let ic_admin = ctx.ic_admin().await?;
             if !ic_admin
-                .propose_print_and_confirm(runner_proposal.cmd.clone(), runner_proposal.opts.clone())
+                .propose_print_and_confirm(
+                    runner_proposal.cmd.clone(),
+                    ProposeOptions {
+                        forum_post_link: Some("[comment]: <> (Link will be added on actual execution)".to_string()),
+                        ..runner_proposal.opts.clone()
+                    },
+                )
                 .await?
             {
                 return Ok(());
             }
 
             let discourse_client = ctx.discourse_client()?;
             let maybe_topic = if let Some(id) = subnet_id {
-                let summary = match (&runner_proposal.opts.summary, &runner_proposal.opts.motivation) {
-                    (Some(s), _) => s,
-                    (None, Some(m)) => m,
-                    _ => {
-                        return Err(anyhow::anyhow!(
-                            "Expected to have `summary` or `motivation` for proposal. Got: {:?}",
-                            runner_proposal
-                        ))
-                    }
+                let body = match (&runner_proposal.opts.motivation, &runner_proposal.opts.summary) {
+                    (Some(motivation), None) => motivation.to_string(),
+                    (Some(motivation), Some(summary)) => format!("{}\nMotivation:\n{}", summary, motivation),
+                    (None, Some(summary)) => summary.to_string(),
+                    (None, None) => anyhow::bail!("Expected to have `motivation` or `summary` for this proposal"),
                 };
-                discourse_client.create_replace_nodes_forum_post(id, summary.to_string()).await?
+
+                discourse_client.create_replace_nodes_forum_post(id, body).await?
             } else {
                 None
             };
@@ -111,7 +114,7 @@ impl ExecutableCommand for Replace {
                             (Some(discourse_response), _) => Some(discourse_response.url.clone()),
                             (None, Some(from_cli_or_auto_formated)) => Some(from_cli_or_auto_formated.clone()),
                             _ => {
-                                warn!("Didn't find a link to forum post from discourse, cli and couldn't auto-format it.");
+                                warn!("Didn't find a link to forum post from discourse or cli and couldn't auto-format it.");
                                 warn!("Will not add forum post to the proposal");
                                 None
                             }
@@ -123,7 +126,7 @@ impl ExecutableCommand for Replace {
 
             if let Some(topic) = maybe_topic {
                 discourse_client
-                    .add_proposal_url_to_post(topic.id, parse_proposal_id_from_governance_response(proposal_response)?)
+                    .add_proposal_url_to_post(topic.update_id, parse_proposal_id_from_governance_response(proposal_response)?)
                     .await?
             }
         }

rs/cli/src/ctx.rs

@@ -274,6 +274,7 @@ impl DreContext {
             // `offline` for discourse client means that it shouldn't try and create posts.
             // It can happen because the tool runs in offline mode, or if its a dry run.
             self.store.is_offline() || self.dry_run,
+            self.discourse_opts.discourse_skip_post_creation,
         )?);
         *self.discourse_client.borrow_mut() = Some(client.clone());
         Ok(client)
@@ -350,6 +351,7 @@ pub mod tests {
                 discourse_api_key: None,
                 discourse_api_url: None,
                 discourse_api_user: None,
+                discourse_skip_post_creation: true,
             },
             discourse_client: RefCell::new(Some(discourse_client)),
         }

rs/cli/src/discourse_client.rs

@@ -1,7 +1,9 @@
-use std::time::Duration;
+use std::{fmt::Display, time::Duration};
 
 use futures::{future::BoxFuture, TryFutureExt};
 use ic_types::PrincipalId;
+use itertools::Itertools;
+use log::warn;
 use mockall::automock;
 use regex::Regex;
 use reqwest::{Client, Method};
@@ -10,9 +12,9 @@ use serde_json::json;
 
 #[automock]
 pub trait DiscourseClient: Sync + Send {
-    fn create_replace_nodes_forum_post(&self, subnet_id: PrincipalId, summary: String) -> BoxFuture<'_, anyhow::Result<Option<DiscourseResponse>>>;
+    fn create_replace_nodes_forum_post(&self, subnet_id: PrincipalId, body: String) -> BoxFuture<'_, anyhow::Result<Option<DiscourseResponse>>>;
 
-    fn add_proposal_url_to_post(&self, post_id: u64, proposal_id: u64) -> BoxFuture<'_, anyhow::Result<()>>;
+    fn add_proposal_url_to_post(&self, post_id: Option<u64>, proposal_id: u64) -> BoxFuture<'_, anyhow::Result<()>>;
 }
 
 pub struct DiscourseClientImp {
@@ -21,10 +23,11 @@ pub struct DiscourseClientImp {
     api_key: String,
     api_user: String,
     offline: bool,
+    skip_forum_post_creation: bool,
 }
 
 impl DiscourseClientImp {
-    pub fn new(url: String, api_key: String, api_user: String, offline: bool) -> anyhow::Result<Self> {
+    pub fn new(url: String, api_key: String, api_user: String, offline: bool, skip_forum_post_creation: bool) -> anyhow::Result<Self> {
         let client = reqwest::Client::builder().timeout(Duration::from_secs(30)).build()?;
 
         Ok(Self {
@@ -33,6 +36,7 @@ impl DiscourseClientImp {
             api_key,
             api_user,
             offline,
+            skip_forum_post_creation,
         })
     }
 
@@ -81,14 +85,14 @@ impl DiscourseClientImp {
             .ok_or(anyhow::anyhow!("Failed to find category with name `{}`", category_name))
     }
 
-    async fn create_topic(&self, title: String, summary: String, category: String, tags: Vec<String>) -> anyhow::Result<DiscourseResponse> {
-        let category = self.get_category_id(category).await?;
+    async fn create_topic(&self, topic: DiscourseTopic) -> anyhow::Result<DiscourseResponse> {
+        let category = self.get_category_id(topic.category).await?;
 
         let payload = json!({
-           "title": title,
+           "title": topic.title,
            "category": category,
-           "raw": summary,
-           "tags": tags
+           "raw": topic.content,
+           "tags": topic.tags
         });
         let payload = serde_json::to_string(&payload)?;
 
@@ -102,7 +106,7 @@ impl DiscourseClientImp {
         };
 
         Ok(DiscourseResponse {
-            id,
+            update_id: Some(id),
             url: format!("{}/t/{}/{}", self.forum_url, topic_slug, topic_id),
         })
     }
@@ -129,42 +133,68 @@ impl DiscourseClientImp {
             .await
             .map(|_resp| ())
     }
+
+    fn request_from_user(&self, err: anyhow::Error, topic: DiscourseTopic) -> anyhow::Result<Option<DiscourseResponse>> {
+        warn!("Received error: {:?}", err);
+        warn!("Please create a topic with the following information");
+        println!("{}", topic);
+        let forum_post_link = dialoguer::Input::<String>::new()
+            .with_prompt("Forum post link")
+            .allow_empty(true)
+            .interact()?;
+        Ok(Some(DiscourseResponse {
+            url: forum_post_link,
+            update_id: None,
+        }))
+    }
 }
 
+const GOVERNANCE_TOPIC: &str = "Governance";
+const SUBNET_MANAGEMENT_TAG: &str = "Subnet-management";
+
 impl DiscourseClient for DiscourseClientImp {
-    fn create_replace_nodes_forum_post(&self, subnet_id: PrincipalId, summary: String) -> BoxFuture<'_, anyhow::Result<Option<DiscourseResponse>>> {
-        Box::pin(async move {
-            if self.offline {
+    fn create_replace_nodes_forum_post(&self, subnet_id: PrincipalId, body: String) -> BoxFuture<'_, anyhow::Result<Option<DiscourseResponse>>> {
+        let subnet_id = subnet_id.to_string();
+        // All principals have a `-` in the string from
+        let (first_part, _rest) = subnet_id.split_once("-").unwrap();
+        let post = DiscourseTopic {
+            title: format!("Replacing nodes in subnet {}", first_part),
+            content: body,
+            tags: vec![SUBNET_MANAGEMENT_TAG.to_string()],
+            category: GOVERNANCE_TOPIC.to_string(),
+        };
+        let post_clone = post.clone();
+
+        let try_call = async move {
+            if self.offline || self.skip_forum_post_creation {
                 return Ok(None);
             }
-            let subnet_id = subnet_id.to_string();
-            let (first_part, _rest) = subnet_id
-                .split_once("-")
-                .ok_or(anyhow::anyhow!("Unexpected principal format `{}`", subnet_id))?;
-            let topic = self
-                .create_topic(
-                    format!("Replacing nodes in subnet {}", first_part),
-                    summary,
-                    "Governance".to_string(),
-                    vec!["Subnet-management".to_string()],
-                )
-                .await?;
+            let topic = self.create_topic(post).await?;
             Ok(Some(topic))
-        })
+        };
+        Box::pin(async move { try_call.await.or_else(|e| self.request_from_user(e, post_clone)) })
     }
 
-    fn add_proposal_url_to_post(&self, post_id: u64, proposal_id: u64) -> BoxFuture<'_, anyhow::Result<()>> {
+    fn add_proposal_url_to_post(&self, post_id: Option<u64>, proposal_id: u64) -> BoxFuture<'_, anyhow::Result<()>> {
         Box::pin(async move {
-            if self.offline {
+            if self.offline || self.skip_forum_post_creation {
                 return Ok(());
             }
 
+            let new_content = format!("Proposal id [{0}](https://dashboard.internetcomputer.org/proposal/{0})", proposal_id);
+            if post_id.is_none() {
+                warn!("Update the forum post with the following text");
+                warn!("{}", new_content);
+                return Ok(());
+            }
+            let post_id = post_id.unwrap();
+
             let content = self.get_post_content(post_id).await?;
             let new_content = format!(
                 r#"{0}
 
-Proposal id [{1}](https://dashboard.internetcomputer.org/proposal/{1})"#,
-                content, proposal_id
+{1}"#,
+                content, new_content
             );
             self.update_post_content(post_id, new_content).await
         })
@@ -190,7 +220,32 @@ pub fn parse_proposal_id_from_governance_response(response: String) -> anyhow::R
 #[derive(Debug)]
 pub struct DiscourseResponse {
     pub url: String,
-    pub id: u64,
+    pub update_id: Option<u64>,
+}
+
+#[derive(Clone)]
+pub struct DiscourseTopic {
+    title: String,
+    content: String,
+    tags: Vec<String>,
+    category: String,
+}
+
+impl Display for DiscourseTopic {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            r#"Title: {}
+Category: {}
+Tags: [{}]
+Content:
+{}"#,
+            self.title,
+            self.category,
+            self.tags.iter().join(", "),
+            self.content
+        )
+    }
 }
 
 #[cfg(test)]

rs/cli/src/ic_admin.rs

@@ -158,7 +158,7 @@ impl IcAdmin for IcAdminImpl {
 
     fn propose_print_and_confirm(&self, cmd: ProposeCommand, opts: ProposeOptions) -> BoxFuture<'_, anyhow::Result<bool>> {
         Box::pin(async move {
-            let _ = self._exec(cmd, opts, true, true, false).await;
+            let _ = self._exec(cmd, opts, true, false, false).await;
 
             if self.proceed_without_confirmation {
                 // Don't ask for confirmation, allow to proceed

rs/cli/src/unit_tests/ctx_init.rs

@@ -52,6 +52,7 @@ async fn get_context(network: &Network, version: IcAdminVersion) -> anyhow::Resu
             discourse_api_key: None,
             discourse_api_url: None,
             discourse_api_user: None,
+            discourse_skip_post_creation: true,
         },
     )
     .await
@@ -193,6 +194,7 @@ async fn get_ctx_for_neuron_test(
             discourse_api_key: None,
             discourse_api_url: None,
             discourse_api_user: None,
+            discourse_skip_post_creation: false,
         },
     )
     .await