Refactor display status messages to refresh entities list (drupal/cloud_dashboard)

@yas

Please review it. Thanks.

@yas

I pushed a new patch to fix by GPT. Thanks.

@ryo-yamashita

Thank you for the patch. Can you please check the following coding standards warning? Thanks

FILE: ...b/modules/contrib/cloud/src/Controller/CloudConfigController.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 41 | WARNING | [x] A comma should follow the last multiline array
    |         |     item. Found: 'vmware'
----------------------------------------------------------------------

@yas

I rebased this patch and push a new commit for coding standard. Thanks.

@ryo-yamashita

Now that you look you want to add updateResourceList() in each ApiController class. In that case, I think we can refactor your patch by introducing CloudContentEntityTrait::updateResourceList() so that we can avoid copy & paste codes. We can consider the other similar methods for such as CloudConfig, LaunchTemplate and Project.

Thanks!

@yas

I pushed a new patch. Thanks.

@ryo-yamashita

Thank you for the update. I posted my comments. Also, please check my previous comment at #3390522-7: Refactor display status messages to refresh entities list (drupal/cloud_dashboard) → .

Thanks

@yas

Based on your comments, I pushed a patch to fix it. Thanks.

@ryo-yamashita

Thank you for the update.

@baldwinlouie

Can you please review the patch? Thanks!

@yas, @ryo-yamashita, Thank you for the patch. I left my comments. @yas, This comment is more a discussion point: https://git.drupalcode.org/project/cloud/-/merge_requests/2039#note_214926

Changing to Needs work

@ryo-yamashita

Thank you for the update. I posted my comments. Thanks!

@ryo-yamashita

Thank you for the update. Can you please check my following comments, too?

• https://git.drupalcode.org/project/cloud/-/merge_requests/2039#note_217265
• https://git.drupalcode.org/project/cloud/-/merge_requests/2039#note_217266

Thanks

@yas

I commented to those comments. Thanks.

@yas

Since there were unnecessary REST APIs that were not accessed by the SPA, I removed them from the YAML file. Thanks.

@ryo-yamashita

Thank you for the update. Now It looks good to me but we need some fixes due to the issues in the original code. I posted my comments. Can you please take a look at the ones? Thanks!

@yas @ryo-yamashita, I've posted my comments.

Please check this comment in particular: https://git.drupalcode.org/project/cloud/-/merge_requests/2039#note_218812 . I was trying to think of ways to avoid adding the new */Rest/*PluginManager classes.

Let me know your thoughts on what I was thinking.

@ryo-yamashita

Thank you for the update. I posted my minor comments.

The patch looks good to me and makes sense to me more especially in @baldwinlouie's comment at https://git.drupalcode.org/project/cloud/-/merge_requests/2039#note_218812.

@baldwinlouie

What do you think?

@ryo-yamashita Thank you for the patch. It's a great refactoring patch. I'm ok with the patch now. I had two small messages that haven't been resolved yet.

https://git.drupalcode.org/project/cloud/-/merge_requests/2039#note_218808

https://git.drupalcode.org/project/cloud/-/merge_requests/2039#note_218809

Let me know your thoughts on these.

@ryo-yamashita

Thank you for your comment. I posted my response. Thanks!

@yas

I added comments for your post. Thanks.

@ryo-yamashita, Thank you for the hard work on this patch. It looks good to me now!

@baldwinlouie

Thank you for your review.

@ryo-yamashita

Thank you for all your efforts. The patch looks good to us now. I'll merge the patch to 5.x and 6.x, and close this issue as Fixed. Thanks!

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