Problem/Motivation

  • Add the function to revoke Openstack Security Group in the SPA.

Issue fork cloud-3291742

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

sekinet created an issue. See original summary.

sekinet’s picture

Status: Active » Needs review

@yas

Could you please review the patch?
Thanks

ryo yamashita’s picture

@sekinet

Thank you for pushing the patch. However, I would like to confirm a few things.

  • In type: 'link', the process is as if a web page with the specified URL is incorporated by dangerouslySetInnerHTML(). Was that implementation required?
  • In EntityDetailPage.tsx, there is a modification to remove "early continue" and add "forEach", may I confirm why?
  • A dependency on RequestStack has been added to OpenStackOperationsService, and a process has been added to retrieve the referrer. Was this the same referrer checking done in the pre-port code? ?

That is all I have to say about reading the code. Thanks.

sekinet’s picture

@ryo-yamashita

Thank you for your comments.

In type: 'link', the process is as if a web page with the specified URL is incorporated by dangerouslySetInnerHTML(). Was that implementation required?

-> It is necessary in order to display the link.
Without this implementation, links would be displayed as strings like <a href ... without this implementation.

In EntityDetailPage.tsx, there is a modification to remove "early continue" and add "forEach", may I confirm why?

-> The reason is to get the index of the record. We need to specify the index in the revoke link.

A dependency on RequestStack has been added to OpenStackOperationsService, and a process has been added to retrieve the referrer. Was this the same referrer checking done in the pre-port code? ?

-> Yes, it is the same.
In the OpenStackSecurityGroupRevokeForm before the port, it was a getRequest.
Since OpenStackOperationsService does not have that method, we are adding RequestStack.

Thanks

ryo yamashita’s picture

@sekinet

Thank you for your answers.

Without this implementation, links would be displayed as strings like

This part can be expressed without dangerouslySetInnerHTML().
A modified example is shown in git diff format.

diff --git a/modules/cloud_dashboard/cloud_dashboard/src/molecules/TableBlock.tsx b/modules/cloud_dashboard/cloud_dashboard/src/molecules/TableBlock.tsx
index 1091997b..4675bf5f 100644
--- a/modules/cloud_dashboard/cloud_dashboard/src/molecules/TableBlock.tsx
+++ b/modules/cloud_dashboard/cloud_dashboard/src/molecules/TableBlock.tsx
@@ -3,6 +3,35 @@ import EntityInfoRecordData from 'model/EntityInfoRecordData';
 import { Table } from 'react-bootstrap';
 import { useTranslation } from 'react-i18next';
 
+const TdBlock = ({ text }: { text: string }) => {
+  try {
+    // For strings that can be parsed as JSON.
+    const temp = JSON.parse(text);
+    if (('type' in temp) && (temp['type'] === 'link')) {
+      const url: string = temp['url'];
+      const label: string = temp['label'];
+
+      return <td className="word-break-all">
+        <a href={url}>{label}</a>
+      </td>;
+    }
+
+    return <td className="word-break-all">
+      {text}
+    </td>;
+  } catch {
+    if (text.includes('\n')) {
+      return <td className="word-break-all">
+        <pre>{text}</pre>
+      </td>;
+    }
+
+    return <td className="word-break-all">
+      {text}
+    </td>;
+  }
+}
+
 /**
  * Block of table view.
  *
@@ -58,13 +87,7 @@ const TableBlock = ({ record }: { record: EntityInfoRecordData }) => {
                 return <tr className={index % 2 === 0 ? 'odd' : 'even'}>
                   {
                     keys.map((key, index2) => {
-                      return <td className="word-break-all" key={index2}>{
-                        r[key].includes('\n')
-                          ? <pre>{r[key]}</pre>
-                          : r[key].includes('<a href')
-                            ? <div dangerouslySetInnerHTML={{ __html: r[key] }}/>
-                            : <>{r[key]}</>
-                      }</td>;
+                      return <TdBlock key={index2} text={r[key]} />;
                     })
                   }
                 </tr>;
diff --git a/modules/cloud_dashboard/cloud_dashboard/src/service/utility.ts b/modules/cloud_dashboard/cloud_dashboard/src/service/utility.ts
index ef388566..c7bd2e30 100644
--- a/modules/cloud_dashboard/cloud_dashboard/src/service/utility.ts
+++ b/modules/cloud_dashboard/cloud_dashboard/src/service/utility.ts
@@ -183,7 +183,11 @@ export const convertDataForUI = (data: any, ec: EntityColumn, dataCache: { [key:
       // source: VmwareVmGuestOsDataProvider.php
       return data in VMWARE_OS_DICT ? VMWARE_OS_DICT[data] : data;
     case 'link':
-        return '<a href="' + ec.url + '">' + ec.label +  '</a>';
+      return JSON.stringify({
+        type: 'link',
+        url: ec.url,
+        label: ec.label
+      });
     default:
       return data;
   }

Note

Within a .tsx format file, return allows JSX to be returned as the return value. In other words, it is used to generate "a" tags.
In addition, the sample code above determines whether or not the "a" tag is a string that you want to use, based on whether or not it is in JSON format.

sekinet’s picture

Status: Needs review » Needs work
sekinet’s picture

Status: Needs work » Needs review

@ryo-yamashita

Thank you for your suggestion. I have fixed it so that dangerouslySetInnerHTML() is not used. Could you please check it?
Thanks

ryo yamashita’s picture

@sekinet

I see no problem with the fix. Thank you for your patch.

yas’s picture

Issue summary: View changes
Status: Needs review » Needs work

@ryo-yamashita

Thank you for your review.

@sekinet

Thank you for the update. I posted my comment. Thanks

sekinet’s picture

Status: Needs work » Needs review

@yas

Thank you for the review. I have fixed the issue as you suggested.
Also, I found that OpenStackOperationsService::validateOpenStackRevoke() and OpenStackOperationsService::validateRestRevoke() were duplicated in their processing, so I have integrated them into OpenStackOperationsService::validatRevoke().
Could you please check it?

Thanks

yas’s picture

@sekinet

Thank you for the update. It looks good to me.

@ryo-yamashita

What do you think?

Thanks

ryo yamashita’s picture

Status: Needs review » Reviewed & tested by the community

@yas @sekinet

It looks good to me, too. Thanks.

yas’s picture

Status: Reviewed & tested by the community » Needs work

@ryo-yamashita

Thank you for your review.

@sekinet

Could you please rebase the patch? Then I'll merge the patch. Thanks!

sekinet’s picture

Status: Needs work » Needs review

@yas

I have rebased the patch. Thanks!

yas’s picture

Status: Needs review » Reviewed & tested by the community

@ryo-yamashita

Thank you for your review.

@sekinet

I'll merge the patch to 4.x and 5.x, and close this issue as Fixed.

  • yas committed 43292d7 on 5.x authored by sekinet
    Issue #3291742 by sekinet, Ryo Yamashita, yas: Add the function to...

  • yas committed 869fb18 on 4.x authored by sekinet
    Issue #3291742 by sekinet, Ryo Yamashita, yas: Add the function to...
yas’s picture

Status: Reviewed & tested by the community » Fixed

@ryo-yamashita

Thank you for your review.

@sekinet

I'll merge the patch to 4.x and 5.x, and close this issue as Fixed.

yas’s picture

Category: Task » Feature request
yas’s picture

Category: Feature request » Task

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.