Comply with Drupal coding standards (30) (phpstan)

@baldwinlouie

Can you please review the patch? Especially I want you to review the code such as $cloud_config_storage = $this->entityTypeManager()->getStorage('cloud_config'); , which looks invalid, and also Error::logException() refactoring.

Please refer the test results by https://git.drupalcode.org/project/cloud/-/pipelines/105757 and ignore the following error message:

 ------ --------------------------------------------------------------------- 
  Line   modules/cloud_service_providers/k8s/k8s.module                       
 ------ --------------------------------------------------------------------- 
  790    Call to static method getOptionsListByRegion() on an unknown class   
         Drupal\Core\DateTime\TimeZoneFormHelper.                             
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols  
 ------ --------------------------------------------------------------------- 

I believe the code return TimeZoneFormHelper::getOptionsListByRegion() is valid however please correct me if I'm wrong.

Thanks!

yas → closed merge request !2241

@baldwinlouie

FYI, I fixed the following error by including the namespace for Drupal\Core\DateTime\TimeZoneFormHelper::getOptionsListByRegion()

------ --------------------------------------------------------------------- 
  Line   modules/cloud_service_providers/k8s/k8s.module                       
 ------ --------------------------------------------------------------------- 
  790    Call to static method getOptionsListByRegion() on an unknown class   
         Drupal\Core\DateTime\TimeZoneFormHelper.                             
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols  
 ------ --------------------------------------------------------------------- 

@baldwinlouie

Thank you for your review. I understood there had the namespace typo. I fixed it, so can you please review the patch again? phpcs and phpstan have been passed successfully :)

Thanks!

@yas, Thank you for working on this patch.

I think the Error::logException() looks good. My concern is the updates for revision handling.

I downloaded and ran this patch to check the revision handling.

The usage of $entity_storage->getRevision($revision_id) causes a 500 error because getRevision() method doesn't exist.

I've been playing with how to use the loadRevision from RevisionableStorageInterface. I think the following modifications should work.

Consider the usage in updateCloudLaunchTemplate in Ec2BatchOperation.php. The following adjustments made it work. You can recreate this error if you try to `Refresh` AWS launch template. The code that refreshes aws launch templates interacts with Revisions.

1. Anywhere you need to load entities with revisions, add PHPDoc comment to add typehinting.

    /** @var \Drupal\Core\Entity\RevisionableStorageInterface $entity_storage */
    $entity_storage = $entity_type_manager->getStorage('cloud_launch_template');

2. Change $entity_storage->getRevision back to $entity_storage->loadRevision($revision_id);.

What do you think?

@baldwinlouie

Thank you for your confirmation and suggestion. I added the type hint to avoid the phpstan warnings. I reverted to loadRevision and refactor to use getRevision() if possible.

Can you please review the patch again? Thanks!

@yas, thank you for updating the patch. I posted my second round of comments. Basically, all `getRevision()` should be changed to `loadRevision()`. Otherwise you will encounter errors such as the following screenshot

To reproduce this error.
1. Go to any AWS Launch Template and click "Edit"
2. Then click the "Revisions" tab.

With the current `getRevision()` code, the "Revisions" tab will error out.

@baldwinlouie

Thank you for your review. I am sorry for my misunderstanding. I confirmed the patch on local environment, and it works. Can you please check the patch again? Thanks!

@yas, thank you for the update! The patch looks good to me now!

@baldwinlouie

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

@baldwinlouie

I found a bug and fixed it. I'll merge the hotfix to 7.x branchand close this issue as Fixed. Thanks!

I found the following error; so I'll fix it.

TypeError: Drupal\aws_cloud\Service\AwsCloud\AwsCloudOperationsService::__construct(): Argument #1 ($entity_type_manager) must be of type Drupal\Core\Entity\EntityTypeManagerInterface, Drupal\cloud\Plugin\cloud\config\CloudConfigPluginManager given, called in /var/www/cloud_orchestrator/docroot/core/lib/Drupal/Component/DependencyInjection/Container.php on line 260 in Drupal\aws_cloud\Service\AwsCloud\AwsCloudOperationsService->__construct() (line 162 of /var/www/cloud_orchestrator/docroot/modules/contrib/cloud/modules/cloud_service_providers/aws_cloud/src/Service/AwsCloud/AwsCloudOperationsService.php).

I tested the patch, so it should work.

@baldwinlouie

I'll merge the hotfix to 7.x branch and close this issue as Fixed. Thanks!

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