From 3f5dea1c6e1130e13e7190bc64b511ac42aea4d7 Mon Sep 17 00:00:00 2001
From: Artjoms Porss <artjom.porsh@gmail.com>
Date: Wed, 9 Oct 2024 18:16:30 +0100
Subject: [PATCH] diplay service provider error message (#2761)

Signed-off-by: aporss <art.porss@yahooinc.com>
---
 .../__snapshots__/ProviderTable.test.js.snap  |  21 +++-
 .../__snapshots__/ServiceList.test.js.snap    |  17 ++-
 .../__snapshots__/ServiceRow.test.js.snap     |   2 +-
 .../__snapshots__/service.test.js.snap        |   5 +-
 ui/src/__tests__/spec/tests/services.spec.js  |  32 ++++++
 ui/src/components/service/ProviderTable.js    | 108 +++++++++---------
 ui/src/components/service/ServiceList.js      |   4 +-
 ui/src/components/service/ServiceRow.js       |   1 +
 8 files changed, 123 insertions(+), 67 deletions(-)

diff --git a/ui/src/__tests__/components/service/__snapshots__/ProviderTable.test.js.snap b/ui/src/__tests__/components/service/__snapshots__/ProviderTable.test.js.snap
index bd71c6b31b7..ab55b869174 100644
--- a/ui/src/__tests__/components/service/__snapshots__/ProviderTable.test.js.snap
+++ b/ui/src/__tests__/components/service/__snapshots__/ProviderTable.test.js.snap
@@ -71,10 +71,16 @@ exports[`ProviderTable should render 1`] = `
   vertical-align: text-bottom;
 }
 
+.emotion-17 {
+  text-align: left;
+  padding: 5px 0 5px 15px;
+  vertical-align: middle;
+}
+
 <td
   class="emotion-0 emotion-1"
   color=""
-  colspan="7"
+  colspan="8"
   data-testid="provider-table"
 >
   <table
@@ -130,6 +136,15 @@ exports[`ProviderTable should render 1`] = `
             </svg>
           </div>
         </td>
+        <td
+          class="emotion-17 emotion-18"
+        >
+          <div
+            data-testid="error-message"
+            name="red600"
+            style="color: rgb(208, 17, 17); display: inline-block;"
+          />
+        </td>
       </tr>
     </tbody>
   </table>
@@ -147,7 +162,5 @@ exports[`ProviderTable should render error if allow api throws error 1`] = `
   data-testid="error-message"
   name="red600"
   style="color: rgb(208, 17, 17); display: inline-block;"
->
-  Failed to fetch template details.
-</div>
+/>
 `;
diff --git a/ui/src/__tests__/components/service/__snapshots__/ServiceList.test.js.snap b/ui/src/__tests__/components/service/__snapshots__/ServiceList.test.js.snap
index f136f7f918c..6146997a2a1 100644
--- a/ui/src/__tests__/components/service/__snapshots__/ServiceList.test.js.snap
+++ b/ui/src/__tests__/components/service/__snapshots__/ServiceList.test.js.snap
@@ -136,6 +136,7 @@ exports[`ServiceList should render add service modal after click 1`] = `
   </div>
   <table
     class="emotion-5 emotion-6"
+    id="services-table"
   >
     <thead>
       <tr>
@@ -264,7 +265,7 @@ exports[`ServiceList should render add service modal after click 1`] = `
             class="emotion-29"
             data-testid="icon"
             height="1.25em"
-            id=""
+            id="openhouse-providers"
             viewBox="0 0 1024 1024"
             width="1.25em"
           >
@@ -451,6 +452,7 @@ exports[`ServiceList should render delete service modal error(other) after click
   </div>
   <table
     class="emotion-5 emotion-6"
+    id="services-table"
   >
     <thead>
       <tr>
@@ -579,7 +581,7 @@ exports[`ServiceList should render delete service modal error(other) after click
             class="emotion-29"
             data-testid="icon"
             height="1.25em"
-            id=""
+            id="openhouse-providers"
             viewBox="0 0 1024 1024"
             width="1.25em"
           >
@@ -756,6 +758,7 @@ exports[`ServiceList should render delete service modal error(refresh) after cli
   </div>
   <table
     class="emotion-5 emotion-6"
+    id="services-table"
   >
     <thead>
       <tr>
@@ -884,7 +887,7 @@ exports[`ServiceList should render delete service modal error(refresh) after cli
             class="emotion-29"
             data-testid="icon"
             height="1.25em"
-            id=""
+            id="openhouse-providers"
             viewBox="0 0 1024 1024"
             width="1.25em"
           >
@@ -1061,6 +1064,7 @@ exports[`ServiceList should render serviceList again after cancel delete 1`] = `
   </div>
   <table
     class="emotion-5 emotion-6"
+    id="services-table"
   >
     <thead>
       <tr>
@@ -1189,7 +1193,7 @@ exports[`ServiceList should render serviceList again after cancel delete 1`] = `
             class="emotion-29"
             data-testid="icon"
             height="1.25em"
-            id=""
+            id="openhouse-providers"
             viewBox="0 0 1024 1024"
             width="1.25em"
           >
@@ -1344,6 +1348,7 @@ exports[`ServiceList should render serviceList again after confirm delete 1`] =
   </div>
   <table
     class="emotion-5 emotion-6"
+    id="services-table"
   >
     <thead>
       <tr>
@@ -1525,6 +1530,7 @@ exports[`ServiceList should render with services 1`] = `
   </div>
   <table
     class="emotion-5 emotion-6"
+    id="services-table"
   >
     <thead>
       <tr>
@@ -1653,7 +1659,7 @@ exports[`ServiceList should render with services 1`] = `
             class="emotion-29"
             data-testid="icon"
             height="1.25em"
-            id=""
+            id="openhouse-providers"
             viewBox="0 0 1024 1024"
             width="1.25em"
           >
@@ -1808,6 +1814,7 @@ exports[`ServiceList should render without services 1`] = `
   </div>
   <table
     class="emotion-5 emotion-6"
+    id="services-table"
   >
     <thead>
       <tr>
diff --git a/ui/src/__tests__/components/service/__snapshots__/ServiceRow.test.js.snap b/ui/src/__tests__/components/service/__snapshots__/ServiceRow.test.js.snap
index 91b2528fede..b781689f7ad 100644
--- a/ui/src/__tests__/components/service/__snapshots__/ServiceRow.test.js.snap
+++ b/ui/src/__tests__/components/service/__snapshots__/ServiceRow.test.js.snap
@@ -178,7 +178,7 @@ exports[`ServiceRow should render row 1`] = `
       class="emotion-8"
       data-testid="icon"
       height="1.25em"
-      id=""
+      id="serviceName-providers"
       viewBox="0 0 1024 1024"
       width="1.25em"
     >
diff --git a/ui/src/__tests__/pages/domain/[domain]/__snapshots__/service.test.js.snap b/ui/src/__tests__/pages/domain/[domain]/__snapshots__/service.test.js.snap
index a87ecc08937..312aba3dbae 100644
--- a/ui/src/__tests__/pages/domain/[domain]/__snapshots__/service.test.js.snap
+++ b/ui/src/__tests__/pages/domain/[domain]/__snapshots__/service.test.js.snap
@@ -1361,6 +1361,7 @@ exports[`ServicePage should render 1`] = `
             </div>
             <table
               class="emotion-103 emotion-104"
+              id="services-table"
             >
               <thead>
                 <tr>
@@ -1489,7 +1490,7 @@ exports[`ServicePage should render 1`] = `
                       class="emotion-84"
                       data-testid="icon"
                       height="1.25em"
-                      id=""
+                      id="bastion-providers"
                       viewBox="0 0 1024 1024"
                       width="1.25em"
                     >
@@ -1612,7 +1613,7 @@ exports[`ServicePage should render 1`] = `
                       class="emotion-84"
                       data-testid="icon"
                       height="1.25em"
-                      id=""
+                      id="openhouse-providers"
                       viewBox="0 0 1024 1024"
                       width="1.25em"
                     >
diff --git a/ui/src/__tests__/spec/tests/services.spec.js b/ui/src/__tests__/spec/tests/services.spec.js
index 04b0bce1b22..90523c62b10 100644
--- a/ui/src/__tests__/spec/tests/services.spec.js
+++ b/ui/src/__tests__/spec/tests/services.spec.js
@@ -81,4 +81,36 @@ describe('services screen tests', () => {
         let modalDeleteButton = await $('button*=Delete');
         await modalDeleteButton.click();
     });
+
+    describe('services screen tests', () => {
+        it('when clicking "Allow" button on a provider without having appropriate authorisation, the error should be displayed to the right of the button', async () => {
+            // open browser
+            await browser.newUser();
+            await browser.url(`/`);
+
+            // select domain
+            let domain = 'athenz.dev.test-non-admin';
+            let testDomain = await $(`a*=${domain}`);
+            await browser.waitUntil(async () => await testDomain.isClickable());
+            await testDomain.click();
+
+            // open Services
+            let servicesDiv = await $('div*=Services');
+            await servicesDiv.click();
+
+            // click Providers
+            let providersButton = await $(`.//*[local-name()="svg" and @id="provider-test-service-providers"]`);
+            await providersButton.click();
+
+            // click Azure provider
+            let awsProviderAllowButton = await $(`td[data-testid="provider-table"]`)
+                .$(`//td[text()="AWS EC2/EKS/Fargate launches instances for the service"]/following-sibling::td//button`);
+            await awsProviderAllowButton.click();
+
+            // warning should appear
+            let warning = await $(`td[data-testid="provider-table"]`)
+                .$(`//td[text()="AWS EC2/EKS/Fargate launches instances for the service"]/following-sibling::td//div[text()="Status: 403. Message: Forbidden"]`);
+            await expect(warning).toHaveText('Status: 403. Message: Forbidden');
+        });
+    });
 })
diff --git a/ui/src/components/service/ProviderTable.js b/ui/src/components/service/ProviderTable.js
index cd2b3aab02d..14865e9f469 100644
--- a/ui/src/components/service/ProviderTable.js
+++ b/ui/src/components/service/ProviderTable.js
@@ -59,6 +59,12 @@ const TableHeadStyledRight = styled.th`
     border-right: none;
 `;
 
+const TableHeadStyledError = styled.th`
+    padding-bottom: 5px;
+    padding: 5px 0 5px 15px;
+    border-right: none;
+`;
+
 const TdStyled = styled.td`
     padding: 20px;
     text-align: left;
@@ -80,6 +86,12 @@ const ProviderTd = styled.td`
     word-break: break-all;
 `;
 
+const ErrorTd = styled.td`
+    text-align: left;
+    padding: 5px 0 5px 15px;
+    vertical-align: middle;
+`;
+
 const AllowTd = styled.td`
     text-align: left;
     padding: 5px 0 5px 15px;
@@ -96,6 +108,7 @@ class ProviderTable extends React.Component {
     constructor(props) {
         super(props);
         this.state = {
+            providerWithError: null,
             errorMessage: null,
         };
     }
@@ -107,9 +120,12 @@ class ProviderTable extends React.Component {
                 this.props.service,
                 provider,
                 this.props._csrf
-            )
+            ).then((data) => {
+                this.setState({ providerWithError: '' });
+            })
             .catch((err) => {
                 this.setState({
+                    providerWithError: provider,
                     errorMessage: RequestUtils.xhrErrorCheckHelper(err),
                 });
             });
@@ -117,72 +133,56 @@ class ProviderTable extends React.Component {
 
     render() {
         let providerContent = [];
-        if (this.state.errorMessage) {
+        providerContent = this.props.allProviders.map((provider) => {
             return (
-                <TdStyled colSpan={7} color={this.props.color}>
-                    <Color name={'red600'}>
-                        Failed to fetch template details.
-                    </Color>
-                </TdStyled>
+                <tr key={provider.id}>
+                    <ProviderTd>{provider.name}</ProviderTd>
+                    <AllowTd colSpan={5}>
+                        {this.props.provider[provider.id] === 'allow' ? (
+                            <AllowDiv>
+                                <Icon
+                                    icon={'checkmark'}
+                                    color={colors.black}
+                                    size={'1.25em'}
+                                    verticalAlign={'text-bottom'}
+                                />
+                            </AllowDiv>
+                        ) : (
+                            <Button
+                                onClick={this.onAllow.bind(
+                                    this,
+                                    provider.id
+                                )}
+                            >
+                                Allow
+                            </Button>
+                        )}
+                    </AllowTd>
+                    <ErrorTd>
+                        <Color name={'red600'}>
+                            {
+                                provider.id === this.state.providerWithError ? this.state.errorMessage : ''
+                            }
+                        </Color>
+                    </ErrorTd>
+                </tr>
             );
-        } else {
-            providerContent = this.props.allProviders.map((provider) => {
-                // if (this.props.provider[provider.id] === 'allow') {
-                return (
-                    <tr key={provider.id}>
-                        <ProviderTd>{provider.name}</ProviderTd>
-                        <AllowTd colSpan={5}>
-                            {this.props.provider[provider.id] === 'allow' ? (
-                                <AllowDiv>
-                                    <Icon
-                                        icon={'checkmark'}
-                                        color={colors.black}
-                                        size={'1.25em'}
-                                        verticalAlign={'text-bottom'}
-                                    />
-                                </AllowDiv>
-                            ) : (
-                                <Button
-                                    onClick={this.onAllow.bind(
-                                        this,
-                                        provider.id
-                                    )}
-                                >
-                                    Allow
-                                </Button>
-                            )}
-                        </AllowTd>
-                    </tr>
-                );
-                //     } else if (this.props.provider[provider.id] === 'not') {
-                //         let onAllow = this.onAllow.bind(this, provider.id);
-                //         return (
-                //             <tr key={provider.id}>
-                //                 <ProviderTd>{provider.name}</ProviderTd>
-                //                 <AllowTd colSpan={5}></AllowTd>
-                //             </tr>
-                //         );
-                //     }
-                // });
-            });
-        }
+        });
 
         return (
             <TdStyled
-                colSpan={7}
+                colSpan={8}
                 color={this.props.color}
                 data-testid='provider-table'
             >
-                {this.state.errorMessage && (
-                    <div>
-                        <Color name={'red600'}>{this.state.errorMessage}</Color>
-                    </div>
-                )}
                 <ProvideTable>
                     <TheadStyled>
                         <tr>
                             <TableHeadStyled>Provider</TableHeadStyled>
                             <TableHeadStyledRight>Status</TableHeadStyledRight>
+                            { this.state.errorMessage  ?
+                                <TableHeadStyledError></TableHeadStyledError> : ''
+                            }
                         </tr>
                     </TheadStyled>
                     <tbody>{providerContent}</tbody>
diff --git a/ui/src/components/service/ServiceList.js b/ui/src/components/service/ServiceList.js
index d6ce6392bbe..70018f844a6 100644
--- a/ui/src/components/service/ServiceList.js
+++ b/ui/src/components/service/ServiceList.js
@@ -223,7 +223,9 @@ class ServiceList extends React.Component {
                         {addService}
                     </div>
                 </AddContainerDiv>
-                <ServiceTable>
+                <ServiceTable
+                    id="services-table"
+                >
                     <thead>
                         <tr>
                             <TableHeadStyled align={left}>
diff --git a/ui/src/components/service/ServiceRow.js b/ui/src/components/service/ServiceRow.js
index e5ebe432eab..539082851a4 100644
--- a/ui/src/components/service/ServiceRow.js
+++ b/ui/src/components/service/ServiceRow.js
@@ -174,6 +174,7 @@ class ServiceRow extends React.Component {
                 </TdStyled>
                 <TdStyled color={color} align={center}>
                     <Icon
+                        id={this.props.serviceName + '-providers'}
                         icon={'cloud'}
                         onClick={this.toggleProviders}
                         color={colors.icons}