From 1c90b660662af672ae9e2d19e9949e4ce2eca0cb Mon Sep 17 00:00:00 2001
From: Nitish Kumar <nitishkumargenius@gmail.com>
Date: Sat, 9 Apr 2022 14:29:44 +0530
Subject: [PATCH] CreateStreamScreen: Show alert when stream name already
 exists.

Show Alert when when we try to create a stream with an existing name.
Refer Discussion:
https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.23M5199/near/1363029

Fixes: #5199
Co-authored-by: Chris Bobbe <cbobbe@zulip.com>
---
 src/streams/CreateStreamScreen.js    | 41 ++++++++++++++++++++++++----
 src/streams/EditStreamCard.js        |  2 +-
 static/translations/messages_en.json |  3 +-
 3 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/src/streams/CreateStreamScreen.js b/src/streams/CreateStreamScreen.js
index db081016e35..81738e25b8e 100644
--- a/src/streams/CreateStreamScreen.js
+++ b/src/streams/CreateStreamScreen.js
@@ -1,15 +1,19 @@
 /* @flow strict-local */
-import React, { useCallback } from 'react';
+import React, { useCallback, useContext } from 'react';
 import type { Node } from 'react';
 
+import { TranslationContext } from '../boot/TranslationProvider';
 import type { RouteProp } from '../react-navigation';
 import type { AppNavigationProp } from '../nav/AppNavigator';
 import * as NavigationService from '../nav/NavigationService';
 import { useSelector } from '../react-redux';
 import { navigateBack } from '../actions';
 import { getAuth } from '../selectors';
+import { getStreamsByName } from '../subscriptions/subscriptionSelectors';
 import Screen from '../common/Screen';
 import EditStreamCard from './EditStreamCard';
+import { showErrorAlert } from '../utils/info';
+import { ApiError } from '../api/apiErrors';
 import * as api from '../api';
 
 type Props = $ReadOnly<{|
@@ -18,14 +22,41 @@ type Props = $ReadOnly<{|
 |}>;
 
 export default function CreateStreamScreen(props: Props): Node {
+  const _ = useContext(TranslationContext);
+
   const auth = useSelector(getAuth);
+  const streamsByName = useSelector(getStreamsByName);
 
   const handleComplete = useCallback(
-    (name: string, description: string, invite_only: boolean) => {
-      api.createStream(auth, { name, description, invite_only });
-      NavigationService.dispatch(navigateBack());
+    async (name: string, description: string, invite_only: boolean) => {
+      // This will miss existing streams that the client can't know about;
+      // for example, a private stream the user can't access. See comment
+      // where we catch an `ApiError`, below.
+      if (streamsByName.has(name)) {
+        showErrorAlert(_('A stream with this name already exists.'));
+        return;
+      }
+
+      try {
+        await api.createStream(auth, { name, description, invite_only });
+        NavigationService.dispatch(navigateBack());
+      } catch (error) {
+        // If the stream already exists but you can't access it (e.g., it's
+        // private), then it won't be in the client's data structures, so
+        // our client-side check above won't stop the request from being
+        // made. In that case, we expect the server to always give an error,
+        // because you can't subscribe to a stream that you can't access.
+        // That error will have:
+        // - error.message: `Unable to access stream (${streamName})`, or a
+        //   translation of that into the user's own language
+        // - error.code: "BAD_REQUEST" (as of server feature level 126;
+        //   possibly the server should be more specific)
+        if (error instanceof ApiError) {
+          showErrorAlert(error.message);
+        }
+      }
     },
-    [auth],
+    [auth, streamsByName, _],
   );
 
   return (
diff --git a/src/streams/EditStreamCard.js b/src/streams/EditStreamCard.js
index 857d6ff8f45..ce5a87692fe 100644
--- a/src/streams/EditStreamCard.js
+++ b/src/streams/EditStreamCard.js
@@ -24,7 +24,7 @@ type Props = $ReadOnly<{|
     description: string,
     invite_only: boolean,
   |},
-  onComplete: (name: string, description: string, invite_only: boolean) => void,
+  onComplete: (name: string, description: string, invite_only: boolean) => void | Promise<void>,
 |}>;
 
 type State = {|
diff --git a/static/translations/messages_en.json b/static/translations/messages_en.json
index 2c70cfb5731..a91f7dc57e4 100644
--- a/static/translations/messages_en.json
+++ b/static/translations/messages_en.json
@@ -300,5 +300,6 @@
   "Copy link to topic": "Copy link to topic",
   "Failed to copy topic link": "Failed to copy topic link",
   "Copy link to stream": "Copy link to stream",
-  "Failed to copy stream link": "Failed to copy stream link"
+  "Failed to copy stream link": "Failed to copy stream link",
+  "A stream with this name already exists.": "A stream with this name already exists."
 }