Comments

jigish.addweb created an issue. See original summary.

jigish.addweb’s picture

Status: Active » Needs review
StatusFileSize
new85.57 KB
yas’s picture

Status: Needs review » Needs work

@jigishaddweb

Thank you for adding the feature. Please refactor:

  1. Use !empty for=== TRUE, !==NULL, isset, and so on.
  2. Add ?: [] to foreach

Looks overlapping lots of logic in between Ec2Service and BatchOperations. It should be refactored by another issue.

yas’s picture

jigish.addweb’s picture

Status: Needs work » Needs review
StatusFileSize
new85.38 KB
new6.61 KB

@yas

Thank you for testing the patch.

I refactored the code. Please test updated patch.

Thanks

yas’s picture

Status: Needs review » Needs work
StatusFileSize
new643.47 KB

@jigishaddweb

Thanks you for the update. Let's reorder the menu tabs

06.png

I had the following error in Recent log messages:

User warning: The configuration name "views.view.openstack_cloud_network_interface" does not match the ID "openstack_network_interface" in Drupal\Core\Config\ConfigInstaller->createConfiguration() (line 373 of /var/www/html/web/core/lib/Drupal/Core/Config/ConfigInstaller.php) 
#0 /var/www/html/web/core/includes/bootstrap.inc(596): _drupal_error_handler_real(512, 'The configurati...', '/var/www/html/w...', 373, Array) 
#1 [internal function]: _drupal_error_handler(512, 'The configurati...', '/var/www/html/w...', 373, Array) 
#2 /var/www/html/web/core/lib/Drupal/Core/Config/ConfigInstaller.php(373): trigger_error('The configurati...', 512) 
#3 /var/www/html/web/core/lib/Drupal/Core/Config/ConfigInstaller.php(137): Drupal\Core\Config\ConfigInstaller->createConfiguration('', Array) 
#4 /var/www/html/web/core/lib/Drupal/Core/ProxyClass/Config/ConfigInstaller.php(75): Drupal\Core\Config\ConfigInstaller->installDefaultConfig('module', 'openstack') 
#5 /var/www/html/web/core/lib/Drupal/Core/Extension/ModuleInstaller.php(288): Drupal\Core\ProxyClass\Config\ConfigInstaller->installDefaultConfig('module', 'openstack')

Could you please check it?

jigish.addweb’s picture

Status: Needs work » Needs review
StatusFileSize
new85.89 KB
new50.09 KB

@yas

Reordered the tab as shown in screenshot. Also, fixed warning.

Please test updated patch.

Thanks

yas’s picture

Status: Needs review » Needs work
StatusFileSize
new981.94 KB
new997.63 KB

@jigishaddweb

Thank you for the update. Please see my comments below:

33.png
40.png

jigish.addweb’s picture

Status: Needs work » Needs review
StatusFileSize
new87.02 KB
new4.35 KB

@yas

I added SecurityGroup link. Attachment is available so we need to keep that fieldset.

Please test updated patch.

Thanks

yas’s picture

Status: Needs review » Needs work
StatusFileSize
new1.12 MB
new892.78 KB

@jigishaddweb

Thank you for the update. I tested and looks good to me now.

06.png

I have a question --- can you hide the attachment fieldset if there are no items?

08.png

jigish.addweb’s picture

Status: Needs work » Needs review
StatusFileSize
new88.27 KB
new1.79 KB

@yas

Thank you for testing the patch.

I hide the Attachment fieldset if there are no items.

Please test new patch file.

Thanks

yas’s picture

Status: Needs review » Reviewed & tested by the community

@jigishaddweb

Thank you for the update. The interdiff was helpful. It looks good to me now. I'll merge the patch to 8.x-1.x and 8.x-2.x and close this issue as Fixed.

  • yas committed aea5615 on 8.x-1.x authored by jigish.addweb
    Issue #3118441 by jigish.addweb, yas: Manage OpenStack Network List of...

  • yas committed 86a710c on 8.x-2.x authored by jigish.addweb
    Issue #3118441 by jigish.addweb, yas: Manage OpenStack Network List of...
yas’s picture

Status: Reviewed & tested by the community » Fixed
yas’s picture

Status: Fixed » Closed (fixed)

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