Problem/Motivation

  • Fix the access control on the Launch template before approval so that the access control is as follows.
    • Launch approved cloud launch template: Only approved Launch template can be launched.
    • Launch cloud launch template: All Launch templates can be launched, including unapproved ones.

Issue fork cloud-3371480

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:

About issue forks

Comments

nakamurarts created an issue. See original summary.

nakamurarts opened merge request !1869

phyllanthus’s picture

phyllanthus commented
Status: Active » Needs review
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Issue summary: View changes

@nakamurarts

Thank you for the fix.

@sekinet

What do you think?

yas’s picture

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

@nakamurarts

The PhpUnit test failed at https://www.drupal.org/pift-ci-job/2705863. Can you please fix it? Thanks

sekinet’s picture

sekinet commented

@nakamurarts @yas

Thank you for the patch.

We may want to confirm this fix with a BDD test. How about adding the following scenario for this purpose?

  • For cloud_admin user, the Launch tab is displayed even before approve
  • General users do not see the Launch tab before approve

Thanks.

phyllanthus’s picture

phyllanthus commented
Status: Needs work » Needs review
yas’s picture

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

@sekinet

Can you please review the patch? Thanks

sekinet’s picture

sekinet commented
Status: Needs review » Needs work

@yas @nakamurarts

Thank you for the patch. I think we should add phpunit tests.

  • For cloud_admin user, the Launch tab is displayed even before approve
  • General users do not see the Launch tab before approve
phyllanthus’s picture

phyllanthus commented
Status: Needs work » Needs review

@yas @sekinet

Thank you all for your reviews.

I added tests based on the suggestion.

Please review, thank you.

yas’s picture

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

@nakamurarts (cc: @sekine)

Thank you for the update. I don't think we should remove the permission launch cloud server template. It means we need to include two test cases scenarios: one for Drupal's administrator, the other one for a generic authenticated user.

yas’s picture

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

@nakamurarts

On my second thought, the test case looks good.

Can you please fix the coding standard error? Thanks

→ https://www.drupal.org/pift-ci-job/2718783

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Category: Task » Bug report
phyllanthus’s picture

phyllanthus commented
Status: Needs work » Needs review

@yas

Thank you for your review. I fixed the coding standard errors.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Title: Fix the access control on the launch template before approval » Fix the access control on the cloud launch template before approval
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

@nakamurarts

Thank you for the update. It looks good to me now. I'll merge the patch to 5.x and 6.x, and close this issue as Fixed.

  • yas committed 3a072f24 on 6.x authored by nakamurarts 
    Issue #3371480 by nakamurarts, yas, sekinet: Fix the access control on...

  • yas committed fefc51fc on 5.x authored by nakamurarts 
    Issue #3371480 by nakamurarts, yas, sekinet: Fix the access control on...
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.