Set up application approval workflow and logic

[jodyheavener]

/dev/web/developer.1password.com/-/issues/1066

Summary

We are working on improvements to the 1Password for Open Source program. This PR continues sets up the workflow and logic to handle when an application is approved.

What's changed?

The focus of the changes here are related to when a reviewer has taken a look at the application, determined it is approved, and applies the status: approved label to the issue.

• Adding the label will kick off a workflow that runs the processor with some additional details about who approved the application
• The processor will run in approve mode, taking in the details about who approved the application
• We'll check if the issue is open, has the correct label, and is still a valid application
• Once everything is confirmed, the application data is added to the repository via commit:
  • We'll use the data we've collected and validated via the issue body and render it as JSON
  • The data will be committed to a new file under the data directory made up of the issue ID and project name, e.g. data/295-foo.json
  • The commit will occur on main
• The issue is then closed with a helpful comment from the bot, indicating who approved the application, what to expect, and some suggestions.

Testing

There are no new unit/integration tests in this PR (more testing setup underway). The manual testing flow to help debug and observe the behaviour when approving in the application processor has been updated. Here's how to test this out:

git checkout jh/approve-merge
make install_deps && make build_processor
./processor approve --test-issue <issue-name> 

You can use this against any of the test issues in the repo, but unless it has the status: approved label it will result in one of the following errors:

Issue closed

You'll receive the error

Error approving application: script run on closed issue

Issue not approved

You'll receive the error

Error approving application: script run on issue that does not have required 'status: approved' label

Application invalid

You'll receive the error

Error approving application: script run on issue with invalid application data:
- ... list of errors ...

It's worth noting that these are all considered edge cases but in theory are all still possible. All of these scenarios will not result in any comment from the bot or label changes.

Happy path

./processor approve --test-issue valid-project-approved

With this command you'll see a few things happen:

1. The application is validated once again.
2. The application contents are committed to the repository:

Output

Commiting the following contents to a file located at "data/6-testdb.json" with the commit message "Added "TestDB" to program":
{
        "account": "testdb.1password.com",
        "project": {
                "name": "TestDB",
                "description": "TestDB is a free and open source, community-based forum software project.",
                "contributors": 1,
                "home_url": "https://github.com/wendyappleed/test-db",
                "repo_url": "https://github.com/wendyappleed/test-db",
                "license_type": "MIT",
                "license_url": "https://github.com/wendyappleed/test-db/blob/main/LICENSE.md",
                "is_event": false,
                "is_team": false
        },
        "applicant": {
                "name": "Wendy Appleseed",
                "email": "wendyappleseed@example.com",
                "role": "Core Maintainer",
                "id": 38230737
        },
        "can_contact": true,
        "approver_id": 123,
        "issue_number": 6,
        "created_at": "2023-07-12T19:49:35Z"
}

3. The bot will post a helpful message

Bot approval message

🎉 Your application has been approved

Congratulations, @approver-username has approved your application! A promotion will be applied to your 1Password account shortly.

If you haven't done so already, we highly recommend implementing a recovery plan for your team in case access for a particular contributor is ever lost. You may add additional core contributors as you see fit.

Finally, we'd love to hear more about your experience using 1Password in your development workflows! Feel free to join us over on the 1Password Developers Slack workspace.

Welcome to the program and happy coding! 🧑‍💻

4. The issue is closed

[schneedotdev]

LGTM! Nice work, @jodyheavener

[michaelAbon1p]

Idiomatic and content tone suggestions, nothing more. I like the structure of this. Feel free to use or ignore my comments as you wish. Ping me for an additional review when you’re done.

[michaelAbon1p]

[style] Using curly apostrophes over straight apostrophes

Suggested change

	If you haven't done so already, we highly recommend implementing a [recovery plan for your team](https://support.1password.com/team-recovery-plan/) in case access for a particular contributor is ever lost. You may add additional core contributors as you see fit.
	If you haven’t done so already, we highly recommend implementing a [recovery plan for your team](https://support.1password.com/team-recovery-plan/) in case access for a particular contributor is ever lost. You may add additional core contributors as you see fit.

There's something about the tone that doesn’t fit well with me. The "highly" or even the "we" and "recommend" all feel out of place. When b5web links out to this team-recovery-plan, it uses sentences like:

To lower the risk of lockout, assign at least one other person to help with account recovery.

1Password can't recover your account for you. Talk to your {{ if .Account.IsFamily }}Family Organizer{{ else }}administrator{{ end }} about implementing a Recovery Plan to make sure you never lose access to your items.

As the only person with permission to recover accounts, your team will rely on you for account recovery. If you forget your account details or misplace your Emergency Kit, you may lose access to your account. To lower risk of lockout, add another person to help with account recovery.

Is there something with a more neutral tone (that's still warm and informal), that doesn't use “we” that we could use here? (Effective writing is hard!) Maybe something like:

Suggested change

	If you haven't done so already, we highly recommend implementing a [recovery plan for your team](https://support.1password.com/team-recovery-plan/) in case access for a particular contributor is ever lost. You may add additional core contributors as you see fit.
	To lower the risk of lockout, [assign at least one other core contributor to help with account recovery](https://support.1password.com/team-recovery-plan/) in case access for a particular contributor is ever lost.

I also realized in writing that sentence that I don't know if you meant "You may add additional core contributors [as members in your 1Password account]" or if you meant "You may add additional core contributors [as recovery group members]". Either way, I like that and I would keep that in there (maybe with the extra information).

[jodyheavener]

This is a good point you raise. This language has been around since the start of the program, but I think I'd like to move away from the "contributor" phrasing since an open source team can be made up of individuals outside that particular title.

To lower the risk of lockout, assign at least one other person to help with account recovery in case access for a particular team member is ever lost. You may add additional team members to your 1Password account, including for account recovery, as you see fit.

[michaelAbon1p]

[Future suggestion] You might consider using spf13/cobra and spf13/pflag, as a way to structure your “main” function, depending on how many more commands and flags. Having a dependency to maintain could be more overhead than this tool deserves, but it could give it the structure it needs.

[jodyheavener]

I was considering this and also flag. For the time being, since these are the only two commands with the singular flag, I'm going to punt and reevaluate down the road.

[michaelAbon1p]

[ignoreable] You might want some tests for this, if anything, to help demonstrate to future maintainers what kinds of inputs and outputs you expect.

[jodyheavener]

I think I'd like to punt on this in favor of more broadly refactoring and testing in a fast-follow.

[michaelAbon1p]

From a code review perspective, this looks great!

[MNThomson]