Separate the Build Plugin Zip action from the PR Comments action

[dkotter]

What?

Instead of having the Build Plugin Zip action being called from the Pull Request Comments action, separate those into two separate actions that are both triggered when a PR is opened.

Why?

I wanted to have the Pull Request Comments action always call the Build Plugin Zip action before proceeding so we could ensure the zip was built properly prior to adding a Playground testing comment to the PR. But as I've attempted to get this working now across a few different PRs, I've run into issues with this approach.

This mostly comes down to permissions (or lack thereof). I originally was using the pull_request_target event as our trigger, which gives higher permission levels to the action workflow. The problem here is when this is the event, GitHub uses the base branch as the reference instead of the PR branch. This meant the zip being built was always based on trunk and never the PR branch, effectively making it worthless.

I switched over to using pull_request and upped the permission level on the job and that seemed to work fine. But in my latest PR, getting permission errors again and from what I've been able to find out, it seems when a PR is opened from a fork, GitHub limits the permissions on that, no matter what permissions you have set in the job (could also be something else I'm doing wrong so open to other opinions here if I'm wrong on this).

How?

Instead of trying to continue to mess with permissions to figure this out, decided it was easiest to just split this into two separate workflows that both are triggered when a PR is opened. So now the Build Plugin Zip action is triggered on the pull_request event, meaning the zip will be built from the PR branch. And the Pull Request Comments action is again triggered from the pull_request_target event, giving it higher permissions to add the Playground testing comment to the PR.

Worth noting the one potential issue here is it's likely the Build Plugin Zip action will take longer than the Pull Request Comments action, meaning a testing comment will be added prior to the zip actually being done. I don't expect this to be an issue in real life, as we're talking just a minute or two difference, but did want to note that.

Testing Instructions

1. Ensure the Playground testing comment is getting added to this PR description.
2. Ensure both the Build Plugin Zip action and Pull Request Comments action succeed

Test using WordPress Playground

The changes in this pull request can be previewed and tested using this WordPress Playground instance:

Click here to test this pull request

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: dkotter <dkotter@git.wordpress.org>
Co-authored-by: jeffpaul <jeffpaul@git.wordpress.org>
Co-authored-by: adamziel <zieladam@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 27.33%. Comparing base (d315ea8) to head (19c293b).
⚠️ Report is 229 commits behind head on develop.

Additional details and impacted files

@@              Coverage Diff              @@
##             develop      #65      +/-   ##
=============================================
- Coverage      31.46%   27.33%   -4.14%     
+ Complexity       176      152      -24     
=============================================
  Files             16       14       -2     
  Lines           1017      900     -117     
=============================================
- Hits             320      246      -74     
+ Misses           697      654      -43     

Flag	Coverage Δ
unit	27.33% <ø> (-4.14%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jeffpaul]

@dkotter any other updates you have planned here? If not, go ahead and merge in so we can start taking benefit of the work in these changes.

[dkotter]

@dkotter any other updates you have planned here? If not, go ahead and merge in so we can start taking benefit of the work in these changes.

Sorry for the delay in response here. I haven't forgotten this, just been focused on other things. This PR does fix the permissions issue we see when a PR is opened from a fork but I am still running into issues where the Playground environment seems to pull in old code instead of the code from the most recent commit. It feels like a caching thing on their end, as a few times now I've clicked on a Playground link and things I expected to see weren't there and then clicking the same link the next day, things worked. But I haven't had time to investigate things further to see if there are changes we need to make here to clear that cache out (or if I'm even correct on that assumption)

[jeffpaul]

@adamziel any chance you or someone from the Playground team could help @dkotter check on this potential caching issue to help resolve this issue?

[adamziel]

Yes, I'll investigate this week. We now have a reusable action for these preview buttons that may help here.

[jeffpaul]

@dkotter I think we can probably punt this to v0.2.0 as its not critical for v0.1.0, yeah?

[dkotter]

@dkotter I think we can probably punt this to v0.2.0 as its not critical for v0.1.0, yeah?

Yep, this is not needed for v0.1.0 (or really any release for that matter)

[adamziel]

I got pulled away sadly and wasn't able to look into this yet. I might be able to next week. Otherwise, see the example repositories with that action already set up.

[dkotter]

I got pulled away sadly and wasn't able to look into this yet. I might be able to next week. Otherwise, see the example repositories with that action already set up.

Thanks for pointing this out. I've opened a new PR to try this out but at the moment, isn't running the second step. It builds the plugin zip fine but the PR Playground Preview workflow isn't triggering. Probably something I've gotten wrong there though I also know GitHub can be a bit weird running new workflows from a PR and you have to merge things in to fully test (or at least I've run into that multiple times before). But if you happen to have some time in the next few weeks to see if what I've setup looks correct, that would be appreciated. Thanks!

[jeffpaul]

@dkotter with #144 merged in, do we still need this PR #65 here or can this be closed out?