1. Sync the Name tag for Elastic IP entity
  2. Add an Action Menu to associate and disassociate the EIP
  3. Change Instance name into hyper link on ElasticIpDisassociateForm
  4. On InstanceEditForm, Once EIP is assigned, it also should show the EIP w/ the name
CommentFileSizeAuthor
#9 3036835-3.patch36.29 KBbaldwinlouie
#6 3036835-2.patch36.23 KBbaldwinlouie
#4 3036835-1.patch108.49 KBbaldwinlouie
#2 3036835.patch35.3 KBbaldwinlouie

Comments

baldwinlouie created an issue. See original summary.

baldwinlouie’s picture

baldwinlouie commented
Status: Active » Needs review
StatusFileSize
new 3036835.patch35.3 KB

Attaching patch that updates to the criteria above.

I also refactored ImageDeleteForm.php and ImageCreateForm.php and abstracted the custom constructor into AwsCloudContentForm.php and AwsDeleteForm.php . With this completed, if we ever have to inject extra classes, we don't have to worry about modifying the custom constructors, which was a pain to maintain. :)

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work

@baldwinlouie

Thank you for providing the patch. That's so nice. I would like to request the following like:

  public function getDescription() {
    $entity = $this->entity;
    $instance_id = $entity->getInstanceId();
    $instance_name = $entity->getName();
    $network_interface_id = $entity->getNetworkInterfaceId();
    $instance_link = $this->entityLinkRenderer->renderViewElement(
      $instance_name ? "$instance_name ($instance_id)" : $instance_id,
      'aws_cloud_instance',
      'instance_id');

    $msg = '<h2>Elastic IP Information:</h2>';
    $msg .= '<ul><li>Instance: ' . $instance_link['#markup'] . '</li><li>Network ID: @network_id</li></ul>';

    return $this->t($msg, [
      ':instance_id' => $instance_link['#markup'],
      '@network_id' => $network_interface_id,
    ]);
  }
baldwinlouie’s picture

baldwinlouie commented
Status: Needs work » Needs review
StatusFileSize
new 3036835-1.patch108.49 KB

@yas, thank you for the feedback. I'm attaching an updated patch.

Status: Needs review » Needs work

The last submitted patch, 4: 3036835-1.patch, failed testing. View results

baldwinlouie’s picture

baldwinlouie commented
Status: Needs work » Needs review
StatusFileSize
new 3036835-2.patch36.23 KB

rerolling the patch.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented

@baldwinlouie

Now it looks so nice！ Thanks！

@xiahoua-guan

What do you think?

xiaohua guan’s picture

xiaohua guan commented

@yas @baldwinlouie

The patch looks OK to me, except that I am wondering whether it is necessary to add CloudContext condition or not.

+    $instances = $this->entityTypeManager->getStorage('aws_cloud_instance')
+      ->loadByProperties([
+        'instance_id' => $instance_id,
+      ]);
baldwinlouie’s picture

baldwinlouie commented
StatusFileSize
new 3036835-3.patch36.29 KB

@yas and @xiaohua-guan, Instance Ids are unique in the system. But as a precaution, I added cloud_context into the query.

xiaohua guan’s picture

xiaohua guan commented
Status: Needs review » Reviewed & tested by the community

@yas @baldwinlouie

It's OK to me now. Thanks. I changed it to RTBC.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented

@xiaohua-guan

Thank you for your review. Then, I'll merge the patch and close this issue as Fixed.

  • yas committed 7cb6aef on 8.x-1.x authored by baldwinlouie 
    Issue #3036835 by baldwinlouie, yas, Xiaohua Guan: Update UI elements...
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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