Add prototype_build_details_comment action

[AliSoftware]

What does it do?

Many of our client apps are configured to generate a Prototype Build (aka Installable Build), then make a comment on the PR with a QR code and some additional info to download the build directly from App Center.

Since the HTML content of those comments are a bit annoying and verbose to hardcode directly in the Fastfile of each of our apps, while they all look the same, I've decided to DRY the generation of this content in a dedicated action that generates a nice HTML table (with the QR code + some metadata) for us, ensuring we use the same nice format in all our client apps.

Checklist before requesting a review

• Run bundle exec rubocop to test for code style violations and recommendations
• Add Unit Tests (aka specs/*_spec.rb) if applicable
• Run bundle exec rspec to run the whole test suite and ensure all your tests pass
• Make sure you added an entry in the CHANGELOG.md file to describe your changes under the approprioate existing ### subsection of the existing ## Trunk section.

[mokagio]

Thank you for this. It's a great DRY improvement. Leverage++

None of my comments are blockers but I think it would be nice to integrate this in a real repo as a test before merging. That work wouldn't be wasted, as could be updated into a PR to integrate the stable version of these changes once released.

[AliSoftware]

I think it would be nice to integrate this in a real repo as a test before merging. That work wouldn't be wasted, as could be updated into a PR to integrate the stable version of these changes once released.

Yep, that has actually already been started… but on one of the Tumblr repo in our GH:E instance https://github.tumblr.net/TumblrMobile/orangina/pull/23867 🙃
I'm planning to make that Tumblr PR point to this branch and start using both the actions I've added as part of #442 + the one added here to refine that PR; once I've proved this worked well for that Tumblr repo, I'm planning on landing this then replicating the improvement to all our client app repos (then P2 about it) some time next sprint.

[AliSoftware]

Testing by starting to use it in a Draft PR in WPiOS: wordpress-mobile/WordPress-iOS#20071

[mokagio]

@AliSoftware

Yep, that has actually already been started… but on one of the Tumblr repo in our GH:E instance github.tumblr.net/TumblrMobile/orangina/pull/23867 🙃

Of course 🤦‍♂️ I didn't put the two things together, should have expected it. Still, thank you for opening a test PR in the WordPress iOS repo as well. Hash-tag working in public, I guess? 🤷‍♂️ 😅

[AliSoftware]

Currently in testing in the following repos to tweak the latest adjustments needed:

• Tumblr-Android: APK uploaded to S3, not App Center
• Tumblr-iOS: Build uploaded to App Center from a Mac agent… but PR comment created via separate CI job run on a different agent (one which has access to Tumblr's private network and GH:E instance), meaning lane_context can't be magically used by the action
• WordPress-Android (APK uploaded to S3, download URL exposed via Cloudfront)
• WordPress-iOS: Build uploaded to App Center, and PR comment created in the same lane, allowing us to have the action use lane_context 🎉

Note: Tests are currently out of sync with the recent refactoring of the API, so will fail. This is expected, as I wanted to focus on adjusting the final API and trying it out in the various client repos above first, before spending the time adjusting the specs only once the API would be confirmed to be fit for all client app's cases above.

TODO:

• Finish testing in the above 4 repos to validate the latest API of this action is a good fit and renders as expected in all the cases
• Update the unit tests (specs) to match the new confirmed API
• Fix the rubocop violations (then ensure the test still pass after adjusting the code to fix them)
• Test one last time in the repos above with the latest changes in the action
• Re-request review, get it approved, release a new version of the toolkit
• Update the Draft PRs above to point to the released version of the toolkit
• P2 it, and profit

[AliSoftware]

Whooops, @mokagio I just saw that I re-requested your review on this PR by accident, while it's not yet ready to re-review (API have been confirmed to be a good fit for client apps, but tests haven't yet been updated with the new API yet nor expanded with all additional use cases… still working on that). And I'm not sure I can cancel a request for re-review on GitHub 🤔

[mokagio]

No worries @AliSoftware .

[AliSoftware]

@mokagio The PR is now ready for final review again 🙂 . Lots of small refactorings and lots of additional unit tests since your last review 😉

[mokagio]

Suggested change

	description: 'The name of the organization in App Center (if you used `appcenter_upload` to distribute your Prototype build)',
	description: 'The name of the organization in App Center #{app_center_auto}',

[AliSoftware]

Nope, in this case it was intentional not to use {app_center_auto} (aka "(will be automatically extracted from lane_context if you used appcenter_upload` to distribute your Prototype build)"), because that's not the case for this property.

In fact, the app_center_org_name is the only value that you have to provide explicitly if you want the action to use App Center.

• Not only is this value not exposed in the lane_context filled by appcenter_upload, unlike all the other properties (the App Center API response includes the app name, but not the app org… who knows why), so this is NOT extracted from lane_context here
• But also, I figured this was a great thing to have a way to tell the action to NOT assume that because appcenter_upload was used that means we want to use its lane_context implicitly (without any way of disabling that) for the comment generation.
  • For example, imagine a case where a fastlane lane (1) creates a beta build and uploads it App Center via appcenter_upload (2) and then creates a Prototype Build, which they do NOT upload to App Center but instead to S3. We don't want the prototype_build_details_comment lane to pick up the lane_context from the appcenter_upload call from (1) while we didn't use App Center for (2).
  • So in the end I decided to consider this fact of "app_center_org_name not being exposed in the lane_context" as a positive rather than a negative, as it allows you too use it as a signaling flag of it you want the action to consider you used App Center or not (instead not having a way to disable the lane_context inference) 🤷