From bbbc9bb1e787ca4de140fad411895f947ec341dc Mon Sep 17 00:00:00 2001 From: Jody Heavener Date: Tue, 26 Mar 2024 10:33:44 -0700 Subject: [PATCH 01/10] Update format of validation error printing --- script/application_test.go | 16 ++++++++-------- script/validator.go | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/script/application_test.go b/script/application_test.go index 37191223..6e408158 100644 --- a/script/application_test.go +++ b/script/application_test.go @@ -7,35 +7,35 @@ import ( ) func errNoProjectName(sectionTitle string) error { - return fmt.Errorf("%s: is missing project name", sectionTitle) + return fmt.Errorf("**%s** is missing project name", sectionTitle) } func errIncomplete(sectionTitle string) error { - return fmt.Errorf("%s: was not completed for application", sectionTitle) + return fmt.Errorf("**%s** was not completed for application", sectionTitle) } func errEmpty(sectionTitle string) error { - return fmt.Errorf("%s: is empty", sectionTitle) + return fmt.Errorf("**%s** is empty", sectionTitle) } func errMustBeChecked(sectionTitle string) error { - return fmt.Errorf("%s: must be checked", sectionTitle) + return fmt.Errorf("**%s** must be checked", sectionTitle) } func errInvalidAccountURL(sectionTitle string) error { - return fmt.Errorf("%s: is invalid 1Password account URL", sectionTitle) + return fmt.Errorf("**%s** is invalid 1Password account URL", sectionTitle) } func errContainsEmoji(sectionTitle string) error { - return fmt.Errorf("%s: cannot contain emoji characters", sectionTitle) + return fmt.Errorf("**%s** cannot contain emoji characters", sectionTitle) } func errParsingNumber(sectionTitle string) error { - return fmt.Errorf("%s: could not be parsed into a number", sectionTitle) + return fmt.Errorf("**%s** could not be parsed into a number", sectionTitle) } func errInvalidURL(sectionTitle string) error { - return fmt.Errorf("%s: is an invalid URL", sectionTitle) + return fmt.Errorf("**%s** is an invalid URL", sectionTitle) } func TestApplication(t *testing.T) { diff --git a/script/validator.go b/script/validator.go index 9dcd247b..1fa5f85c 100644 --- a/script/validator.go +++ b/script/validator.go @@ -30,7 +30,7 @@ type ValidationError struct { type ValidatorCallback func(string) (bool, string, string) func (e *ValidationError) Error() string { - return fmt.Sprintf("%s: %s", e.Section, e.Message) + return fmt.Sprintf("**%s** %s", e.Section, e.Message) } type Validator struct { From d9c0b87a241dd4cdbe792ace2d968a75682c25eb Mon Sep 17 00:00:00 2001 From: Jody Heavener Date: Tue, 26 Mar 2024 11:08:58 -0700 Subject: [PATCH 02/10] Update review process for bot to modify labels and leave comments on issue, based on issue status and validity --- script/reviewer.go | 84 +++++++++++++++++-- .../test-issues/valid-project-reviewing.json | 60 +++++++++++++ 2 files changed, 139 insertions(+), 5 deletions(-) create mode 100644 script/test-issues/valid-project-reviewing.json diff --git a/script/reviewer.go b/script/reviewer.go index 04c1371f..33d3b391 100644 --- a/script/reviewer.go +++ b/script/reviewer.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "log" ) @@ -19,11 +20,84 @@ func (r *Reviewer) Review() { r.application.Parse(r.gitHub.Issue) - if isTestingIssue() { - if r.application.IsValid() { - debugMessage("Application has no problems") - } else { - debugMessage("Application problems:", r.application.RenderProblems()) + status := r.getStatus() + r.updateLabels(status) + r.createComment(status) +} + +func (r *Reviewer) getStatus() string { + if *r.gitHub.Issue.State == "closed" { + return "closed" + } else if r.gitHub.IssueHasLabel(LabelStatusApproved) { + return "approved" + } else if r.gitHub.IssueHasLabel(LabelStatusReviewing) { + return "reviewing" + } else if r.gitHub.IssueHasLabel(LabelStatusInvalid) { + return "invalid" + } else { + return "new" + } +} + +func (r *Reviewer) createComment(status string) { + title := "" + body := "" + details := fmt.Sprintf("
\nApplication data...\n\n```json\n%s\n```\n
", r.application.GetData()) + + if status == "closed" { + body = "Oops! This application is closed can no longer be processed. If this is an error, please reach out to [opensource@1password.com](mailto:opensource@1password.com)." + } else if status == "approved" { + body = "Oops! This application has been updated but has already been approved and can no longer be processed. If this is an error, please reach out to [opensource@1password.com](mailto:opensource@1password.com)." + } else if status == "reviewing" && r.application.IsValid() { + title = "### 👍 Application still valid" + body = fmt.Sprintf("\n\n%s\n\nWe've processed your updated application and everything still looks good.", details) + } else if r.application.IsValid() { + title = "### ✅ Your application is valid" + body = fmt.Sprintf("\n\n%s\n\nThanks for applying! Next step: our team will review your application and may have follow-up questions. You can still make changes to your application and we'll process it again.", details) + } else { + title = "### ❌ Your application needs some work" + body = fmt.Sprintf("\n\n%s\n\nThe following issues need to be addressed:\n\n%s", details, r.application.RenderProblems()) + } + + r.gitHub.CreateIssueComment(fmt.Sprintf("%s%s", title, body)) +} + +func (r *Reviewer) updateLabels(status string) { + if status == "approved" || status == "closed" { + return + } + + if status == "invalid" && r.application.IsValid() { + if err := r.gitHub.RemoveIssueLabel(LabelStatusInvalid); err != nil { + r.printErrorAndExit( + fmt.Errorf("could not remove issue label '%s': %s", LabelStatusInvalid, err.Error()), + ) + } + } + + if r.application.IsValid() { + if status != "reviewing" { + if err := r.gitHub.AddIssueLabel(LabelStatusReviewing); err != nil { + r.printErrorAndExit( + fmt.Errorf("could not add issue label '%s': %s", LabelStatusReviewing, err.Error()), + ) + } + } + } else { + if status != "invalid" { + if err := r.gitHub.AddIssueLabel(LabelStatusInvalid); err != nil { + r.printErrorAndExit( + fmt.Errorf("could not add issue label '%s': %s", LabelStatusInvalid, err.Error()), + ) + } + } + + if status == "reviewing" { + if err := r.gitHub.RemoveIssueLabel(LabelStatusReviewing); err != nil { + r.printErrorAndExit( + fmt.Errorf("could not remove issue label '%s': %s", LabelStatusReviewing, err.Error()), + ) + } } } } diff --git a/script/test-issues/valid-project-reviewing.json b/script/test-issues/valid-project-reviewing.json new file mode 100644 index 00000000..80042b67 --- /dev/null +++ b/script/test-issues/valid-project-reviewing.json @@ -0,0 +1,60 @@ +{ + "id": 1801650328, + "number": 6, + "state": "open", + "locked": false, + "title": "Application for TestDB", + "body": "### Account URL\n\ntestdb.1password.com\n\n### Non-commercial confirmation\n\n- [X] No, this account won't be used for commercial activity\n\n### Team application\n\n- [ ] Yes, this application is for a team\n\n### Event application\n\n- [ ] Yes, this application is for an event\n\n### Project name\n\nTestDB\n\n### Short description\n\nTestDB is a free and open source, community-based forum software project.\n\n### Number of team members/core contributors\n\n1\n\n### Homepage URL\n\nhttps://github.com/wendyappleed/test-db\n\n### Repository URL\n\nhttps://github.com/wendyappleed/test-db\n\n### License type\n\nMIT\n\n### License URL\n\nhttps://github.com/wendyappleed/test-db/blob/main/LICENSE.md\n\n### Age confirmation\n\n- [X] Yes, this project is at least 30 days old\n\n### Name\n\nWendy Appleseed\n\n### Email\n\nwendyappleseed@example.com\n\n### Project role\n\nCore Maintainer\n\n### Profile or website\n\nhttps://github.com/wendyappleseed/\n\n### Can we contact you?\n\n- [X] Yes, you may contact me\n\n### Additional comments\n\nThank you!", + "user": { + "login": "wendyappleseed", + "id": 38230737, + "node_id": "MDQ6VXNlcjYzOTIwNDk=", + "avatar_url": "https://avatars.githubusercontent.com/u/38230737?v=4", + "html_url": "https://github.com/wendyappleseed", + "gravatar_id": "", + "type": "User", + "site_admin": false, + "url": "https://api.github.com/users/wendyappleseed", + "events_url": "https://api.github.com/users/wendyappleseed/events{/privacy}", + "following_url": "https://api.github.com/users/wendyappleseed/following{/other_user}", + "followers_url": "https://api.github.com/users/wendyappleseed/followers", + "gists_url": "https://api.github.com/users/wendyappleseed/gists{/gist_id}", + "organizations_url": "https://api.github.com/users/wendyappleseed/orgs", + "received_events_url": "https://api.github.com/users/wendyappleseed/received_events", + "repos_url": "https://api.github.com/users/wendyappleseed/repos", + "starred_url": "https://api.github.com/users/wendyappleseed/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/wendyappleseed/subscriptions" + }, + "labels": [ + { + "id": 5728067083, + "url": "https://api.github.com/repos/1Password/1password-teams-open-source/labels/status:%20reviewing", + "name": "status: reviewing", + "color": "0052CC", + "description": "The application is being reviewed", + "default": false, + "node_id": "LA_kwDOJ6JE6M8AAAABVWteCw" + } + ], + "comments": 11, + "closed_at": "2023-07-13T05:03:51Z", + "created_at": "2023-07-12T19:49:35Z", + "updated_at": "2023-07-13T05:03:51Z", + "url": "https://api.github.com/repos/1Password/1password-teams-open-source/issues/6", + "html_url": "https://github.com/wendyappleseed/1password-teams-open-source/issues/6", + "comments_url": "https://api.github.com/repos/1Password/1password-teams-open-source/issues/6/comments", + "events_url": "https://api.github.com/repos/1Password/1password-teams-open-source/issues/6/events", + "labels_url": "https://api.github.com/repos/1Password/1password-teams-open-source/issues/6/labels{/name}", + "repository_url": "https://api.github.com/repos/1Password/1password-teams-open-source", + "reactions": { + "total_count": 0, + "+1": 0, + "-1": 0, + "laugh": 0, + "confused": 0, + "heart": 0, + "hooray": 0, + "url": "https://api.github.com/repos/1Password/1password-teams-open-source/issues/6/reactions" + }, + "node_id": "I_kwDOJ6JE6M5rYwCY" +} From 14a106cab12a9dd399063a875a16fc18f92ac2de Mon Sep 17 00:00:00 2001 From: Jody Heavener Date: Wed, 27 Mar 2024 08:17:02 -0700 Subject: [PATCH 03/10] Update representation of status values from strings to int constants using custom Status type --- script/reviewer.go | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/script/reviewer.go b/script/reviewer.go index 33d3b391..f199c885 100644 --- a/script/reviewer.go +++ b/script/reviewer.go @@ -5,6 +5,16 @@ import ( "log" ) +type Status int + +const ( + Closed Status = iota + Approved + Reviewing + Invalid + New +) + type Reviewer struct { gitHub GitHub application Application @@ -25,30 +35,30 @@ func (r *Reviewer) Review() { r.createComment(status) } -func (r *Reviewer) getStatus() string { +func (r *Reviewer) getStatus() Status { if *r.gitHub.Issue.State == "closed" { - return "closed" + return Closed } else if r.gitHub.IssueHasLabel(LabelStatusApproved) { - return "approved" + return Approved } else if r.gitHub.IssueHasLabel(LabelStatusReviewing) { - return "reviewing" + return Reviewing } else if r.gitHub.IssueHasLabel(LabelStatusInvalid) { - return "invalid" + return Invalid } else { - return "new" + return New } } -func (r *Reviewer) createComment(status string) { +func (r *Reviewer) createComment(status Status) { title := "" body := "" details := fmt.Sprintf("
\nApplication data...\n\n```json\n%s\n```\n
", r.application.GetData()) - if status == "closed" { + if status == Closed { body = "Oops! This application is closed can no longer be processed. If this is an error, please reach out to [opensource@1password.com](mailto:opensource@1password.com)." - } else if status == "approved" { + } else if status == Approved { body = "Oops! This application has been updated but has already been approved and can no longer be processed. If this is an error, please reach out to [opensource@1password.com](mailto:opensource@1password.com)." - } else if status == "reviewing" && r.application.IsValid() { + } else if status == Reviewing && r.application.IsValid() { title = "### 👍 Application still valid" body = fmt.Sprintf("\n\n%s\n\nWe've processed your updated application and everything still looks good.", details) } else if r.application.IsValid() { @@ -62,12 +72,12 @@ func (r *Reviewer) createComment(status string) { r.gitHub.CreateIssueComment(fmt.Sprintf("%s%s", title, body)) } -func (r *Reviewer) updateLabels(status string) { - if status == "approved" || status == "closed" { +func (r *Reviewer) updateLabels(status Status) { + if status == Approved || status == Closed { return } - if status == "invalid" && r.application.IsValid() { + if status == Invalid && r.application.IsValid() { if err := r.gitHub.RemoveIssueLabel(LabelStatusInvalid); err != nil { r.printErrorAndExit( fmt.Errorf("could not remove issue label '%s': %s", LabelStatusInvalid, err.Error()), @@ -76,7 +86,7 @@ func (r *Reviewer) updateLabels(status string) { } if r.application.IsValid() { - if status != "reviewing" { + if status != Reviewing { if err := r.gitHub.AddIssueLabel(LabelStatusReviewing); err != nil { r.printErrorAndExit( fmt.Errorf("could not add issue label '%s': %s", LabelStatusReviewing, err.Error()), @@ -84,7 +94,7 @@ func (r *Reviewer) updateLabels(status string) { } } } else { - if status != "invalid" { + if status != Invalid { if err := r.gitHub.AddIssueLabel(LabelStatusInvalid); err != nil { r.printErrorAndExit( fmt.Errorf("could not add issue label '%s': %s", LabelStatusInvalid, err.Error()), @@ -92,7 +102,7 @@ func (r *Reviewer) updateLabels(status string) { } } - if status == "reviewing" { + if status == Reviewing { if err := r.gitHub.RemoveIssueLabel(LabelStatusReviewing); err != nil { r.printErrorAndExit( fmt.Errorf("could not remove issue label '%s': %s", LabelStatusReviewing, err.Error()), From a612cdf59bf71b3d7fed8546bd9806ea22b9193d Mon Sep 17 00:00:00 2001 From: Jody Heavener Date: Wed, 27 Mar 2024 08:53:27 -0700 Subject: [PATCH 04/10] Copy updates --- script/reviewer.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/script/reviewer.go b/script/reviewer.go index f199c885..726002ba 100644 --- a/script/reviewer.go +++ b/script/reviewer.go @@ -53,19 +53,21 @@ func (r *Reviewer) createComment(status Status) { title := "" body := "" details := fmt.Sprintf("
\nApplication data...\n\n```json\n%s\n```\n
", r.application.GetData()) + // TODO: replace FILE_NAME with Application.FileName once available + dataPath := fmt.Sprintf("https://github.com/1Password/1password-teams-open-source/blob/main/data/%s", "FILE_NAME") if status == Closed { - body = "Oops! This application is closed can no longer be processed. If this is an error, please reach out to [opensource@1password.com](mailto:opensource@1password.com)." + body = "This application is closed and changes will not be reviewed. If this is an error, contact us at [opensource@1password.com](mailto:opensource@1password.com)." } else if status == Approved { - body = "Oops! This application has been updated but has already been approved and can no longer be processed. If this is an error, please reach out to [opensource@1password.com](mailto:opensource@1password.com)." + body = fmt.Sprintf("This application has already been approved and changes will not be reviewed. If you would like to modify the details of your application, submit a pull request against the stored [application data](%s). If this is an error, contact us at [opensource@1password.com](mailto:opensource@1password.com).", dataPath) } else if status == Reviewing && r.application.IsValid() { title = "### 👍 Application still valid" - body = fmt.Sprintf("\n\n%s\n\nWe've processed your updated application and everything still looks good.", details) + body = fmt.Sprintf("\n\n%s\n\nWe’ve evaluated your updated application and it is still valid.", details) } else if r.application.IsValid() { title = "### ✅ Your application is valid" - body = fmt.Sprintf("\n\n%s\n\nThanks for applying! Next step: our team will review your application and may have follow-up questions. You can still make changes to your application and we'll process it again.", details) + body = fmt.Sprintf("\n\n%s\n\nThanks for applying! Next step: our team will review your application and may have follow-up questions. You can still make changes to your application and it’ll be re-evaluated.", details) } else { - title = "### ❌ Your application needs some work" + title = "### ❌ Your application is invalid" body = fmt.Sprintf("\n\n%s\n\nThe following issues need to be addressed:\n\n%s", details, r.application.RenderProblems()) } From 65ddc72ea224872885b2f1130863f41344ad8686 Mon Sep 17 00:00:00 2001 From: Jody Heavener Date: Wed, 27 Mar 2024 08:53:47 -0700 Subject: [PATCH 05/10] Update position of s tatus/validity check when creating comment --- script/reviewer.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/script/reviewer.go b/script/reviewer.go index 726002ba..87d9e79d 100644 --- a/script/reviewer.go +++ b/script/reviewer.go @@ -60,12 +60,14 @@ func (r *Reviewer) createComment(status Status) { body = "This application is closed and changes will not be reviewed. If this is an error, contact us at [opensource@1password.com](mailto:opensource@1password.com)." } else if status == Approved { body = fmt.Sprintf("This application has already been approved and changes will not be reviewed. If you would like to modify the details of your application, submit a pull request against the stored [application data](%s). If this is an error, contact us at [opensource@1password.com](mailto:opensource@1password.com).", dataPath) - } else if status == Reviewing && r.application.IsValid() { - title = "### 👍 Application still valid" - body = fmt.Sprintf("\n\n%s\n\nWe’ve evaluated your updated application and it is still valid.", details) } else if r.application.IsValid() { - title = "### ✅ Your application is valid" - body = fmt.Sprintf("\n\n%s\n\nThanks for applying! Next step: our team will review your application and may have follow-up questions. You can still make changes to your application and it’ll be re-evaluated.", details) + if status == Reviewing { + title = "### 👍 Application still valid" + body = fmt.Sprintf("\n\n%s\n\nWe’ve evaluated your updated application and it is still valid.", details) + } else { + title = "### ✅ Your application is valid" + body = fmt.Sprintf("\n\n%s\n\nThanks for applying! Next step: our team will review your application and may have follow-up questions. You can still make changes to your application and it’ll be re-evaluated.", details) + } } else { title = "### ❌ Your application is invalid" body = fmt.Sprintf("\n\n%s\n\nThe following issues need to be addressed:\n\n%s", details, r.application.RenderProblems()) From 7c5732971bc3efbe3d091747ed9feaa3ff758ade Mon Sep 17 00:00:00 2001 From: Jody Heavener Date: Wed, 27 Mar 2024 08:57:55 -0700 Subject: [PATCH 06/10] Copy updates --- script/reviewer.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/script/reviewer.go b/script/reviewer.go index 87d9e79d..81f2cddf 100644 --- a/script/reviewer.go +++ b/script/reviewer.go @@ -57,20 +57,20 @@ func (r *Reviewer) createComment(status Status) { dataPath := fmt.Sprintf("https://github.com/1Password/1password-teams-open-source/blob/main/data/%s", "FILE_NAME") if status == Closed { - body = "This application is closed and changes will not be reviewed. If this is an error, contact us at [opensource@1password.com](mailto:opensource@1password.com)." + body = "This application is closed and changes will not be reviewed. If you have any questions, contact us at [opensource@1password.com](mailto:opensource@1password.com)." } else if status == Approved { - body = fmt.Sprintf("This application has already been approved and changes will not be reviewed. If you would like to modify the details of your application, submit a pull request against the stored [application data](%s). If this is an error, contact us at [opensource@1password.com](mailto:opensource@1password.com).", dataPath) + body = fmt.Sprintf("This application has already been approved and changes will not be reviewed. If you would like to modify the details of your application, submit a pull request against the stored [application data](%s). If you have any questions, contact us at [opensource@1password.com](mailto:opensource@1password.com).", dataPath) } else if r.application.IsValid() { if status == Reviewing { title = "### 👍 Application still valid" - body = fmt.Sprintf("\n\n%s\n\nWe’ve evaluated your updated application and it is still valid.", details) + body = fmt.Sprintf("\n\n%s\n\nWe’ve run our automated pre-checks and your updated application is still valid.", details) } else { title = "### ✅ Your application is valid" - body = fmt.Sprintf("\n\n%s\n\nThanks for applying! Next step: our team will review your application and may have follow-up questions. You can still make changes to your application and it’ll be re-evaluated.", details) + body = fmt.Sprintf("\n\n%s\n\nThanks for applying! Our automated pre-checks have determined your application is valid. Next step: our team will review your application and may have follow-up questions. You can still make changes to your application and it’ll be re-evaluated.", details) } } else { title = "### ❌ Your application is invalid" - body = fmt.Sprintf("\n\n%s\n\nThe following issues need to be addressed:\n\n%s", details, r.application.RenderProblems()) + body = fmt.Sprintf("\n\n%s\n\nOur automated pre-checks have detected the following issues:\n\n%s", details, r.application.RenderProblems()) } r.gitHub.CreateIssueComment(fmt.Sprintf("%s%s", title, body)) From 56e5a1372fd8cef2470fee34e255bc416a48fb5f Mon Sep 17 00:00:00 2001 From: Jody Heavener Date: Thu, 28 Mar 2024 10:21:01 -0700 Subject: [PATCH 07/10] Sanitize all user input --- script/application_test.go | 4 ++ .../valid-project-character-test.json | 49 +++++++++++++++++++ script/validator.go | 10 ++-- script/validator_test.go | 2 +- 4 files changed, 59 insertions(+), 6 deletions(-) create mode 100644 script/test-issues/valid-project-character-test.json diff --git a/script/application_test.go b/script/application_test.go index 6e408158..34e942f4 100644 --- a/script/application_test.go +++ b/script/application_test.go @@ -71,6 +71,10 @@ func TestApplication(t *testing.T) { name: "event", expectedValid: true, }, + { + name: "project-character-test", + expectedValid: true, + }, { name: "empty-body", expectedValid: false, diff --git a/script/test-issues/valid-project-character-test.json b/script/test-issues/valid-project-character-test.json new file mode 100644 index 00000000..59363858 --- /dev/null +++ b/script/test-issues/valid-project-character-test.json @@ -0,0 +1,49 @@ +{ + "id": 1801650328, + "number": 6, + "state": "open", + "locked": false, + "title": "Application for TestDB", + "body": "### Account URL\n\n`testdb.1password.com`\n\n### Non-commercial confirmation\n\n- [X] No, this account won't be used for commercial activity\n\n### 👨‍💻 Team application\n\n- [X] Yes, this application is for a team\n\n### Event application\n\n- [ ] Yes, this application is for an event\n\n### Project name\n\n`_TestDB_\n\n- test``\n\n### Short description\n\nTestDB is a free and open source, community-based forum software project.```\n\n## **This is a test comment** \n\n### Number of team members/core contributors\n\n1\n\n### Homepage URL\n\nhttps://github.com/wendyappleed/test-db\n\n### Repository URL\n\nhttps://github.com/wendyappleed/test-db\n\n### License type\n\nMIT\n\n### License URL\n\nhttps://github.com/wendyappleed/test-db/blob/main/LICENSE.md\n\n### Age confirmation\n\n- [X] Yes, this project is at least 30 days old\n\n### Name\n\nWendy Appleseed\n\n### Email\n\nwendyappleseed@example.com\n\n### Project role\n\nCore Maintainer\n\n### Profile or website\n\nhttps://github.com/wendyappleseed/\n\n### Can we contact you?\n\n- [X] Yes, you may contact me\n\n### Additional comments\n\n Thank you!", + "user": { + "login": "wendyappleseed", + "id": 38230737, + "node_id": "MDQ6VXNlcjYzOTIwNDk=", + "avatar_url": "https://avatars.githubusercontent.com/u/38230737?v=4", + "html_url": "https://github.com/wendyappleseed", + "gravatar_id": "", + "type": "User", + "site_admin": false, + "url": "https://api.github.com/users/wendyappleseed", + "events_url": "https://api.github.com/users/wendyappleseed/events{/privacy}", + "following_url": "https://api.github.com/users/wendyappleseed/following{/other_user}", + "followers_url": "https://api.github.com/users/wendyappleseed/followers", + "gists_url": "https://api.github.com/users/wendyappleseed/gists{/gist_id}", + "organizations_url": "https://api.github.com/users/wendyappleseed/orgs", + "received_events_url": "https://api.github.com/users/wendyappleseed/received_events", + "repos_url": "https://api.github.com/users/wendyappleseed/repos", + "starred_url": "https://api.github.com/users/wendyappleseed/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/wendyappleseed/subscriptions" + }, + "comments": 11, + "closed_at": "2023-07-13T05:03:51Z", + "created_at": "2023-07-12T19:49:35Z", + "updated_at": "2023-07-13T05:03:51Z", + "url": "https://api.github.com/repos/1Password/1password-teams-open-source/issues/6", + "html_url": "https://github.com/wendyappleseed/1password-teams-open-source/issues/6", + "comments_url": "https://api.github.com/repos/1Password/1password-teams-open-source/issues/6/comments", + "events_url": "https://api.github.com/repos/1Password/1password-teams-open-source/issues/6/events", + "labels_url": "https://api.github.com/repos/1Password/1password-teams-open-source/issues/6/labels{/name}", + "repository_url": "https://api.github.com/repos/1Password/1password-teams-open-source", + "reactions": { + "total_count": 0, + "+1": 0, + "-1": 0, + "laugh": 0, + "confused": 0, + "heart": 0, + "hooray": 0, + "url": "https://api.github.com/repos/1Password/1password-teams-open-source/issues/6/reactions" + }, + "node_id": "I_kwDOJ6JE6M5rYwCY" +} diff --git a/script/validator.go b/script/validator.go index 1fa5f85c..b783a858 100644 --- a/script/validator.go +++ b/script/validator.go @@ -69,11 +69,7 @@ func ParseInput(value string) (bool, string, string) { return true, "", "" } - return true, value, "" -} - -func ParsePlainString(value string) (bool, string, string) { - // strip all formattig, except for newlines + // strip all formatting, except for newlines html := blackfriday.Run([]byte(value)) doc, err := goquery.NewDocumentFromReader(bytes.NewReader(html)) if err != nil { @@ -81,6 +77,10 @@ func ParsePlainString(value string) (bool, string, string) { } value = strings.TrimSpace(doc.Text()) + return true, value, "" +} + +func ParsePlainString(value string) (bool, string, string) { if urlRegex.MatchString(value) { return false, value, "cannot contain URLs" } diff --git a/script/validator_test.go b/script/validator_test.go index 3a458240..cbeb9a65 100644 --- a/script/validator_test.go +++ b/script/validator_test.go @@ -30,6 +30,7 @@ func TestParseInput(t *testing.T) { {"_No response_", true, "", ""}, {"None", true, "", ""}, {"hello", true, "hello", ""}, + {"Testing formatting and link stripping", true, "Testing formatting and link stripping", ""}, } runValidationTests(t, testCases, ParseInput, "ParseInput") } @@ -39,7 +40,6 @@ func TestParsePlainString(t *testing.T) { {"", true, "", ""}, {"Hello world", true, "Hello world", ""}, {"👋 howdy", false, "👋 howdy", "cannot contain emoji characters"}, - {"Testing formatting and link stripping", true, "Testing formatting and link stripping", ""}, } runValidationTests(t, testCases, ParsePlainString, "ParsePlainString") } From f52b011b39590fe45f2fe67098e5cf3e9cfc2ed0 Mon Sep 17 00:00:00 2001 From: Jody Heavener Date: Thu, 28 Mar 2024 10:40:06 -0700 Subject: [PATCH 08/10] Move validity check in updateLabels --- script/reviewer.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/script/reviewer.go b/script/reviewer.go index 81f2cddf..60cd3cfc 100644 --- a/script/reviewer.go +++ b/script/reviewer.go @@ -81,15 +81,15 @@ func (r *Reviewer) updateLabels(status Status) { return } - if status == Invalid && r.application.IsValid() { - if err := r.gitHub.RemoveIssueLabel(LabelStatusInvalid); err != nil { - r.printErrorAndExit( - fmt.Errorf("could not remove issue label '%s': %s", LabelStatusInvalid, err.Error()), - ) + if r.application.IsValid() { + if status == Invalid { + if err := r.gitHub.RemoveIssueLabel(LabelStatusInvalid); err != nil { + r.printErrorAndExit( + fmt.Errorf("could not remove issue label '%s': %s", LabelStatusInvalid, err.Error()), + ) + } } - } - if r.application.IsValid() { if status != Reviewing { if err := r.gitHub.AddIssueLabel(LabelStatusReviewing); err != nil { r.printErrorAndExit( From 53cba3220d4f9f09b936d5d425ee35a5d5f86282 Mon Sep 17 00:00:00 2001 From: Jody Heavener Date: Thu, 28 Mar 2024 10:45:19 -0700 Subject: [PATCH 09/10] Update copy when application is invalid --- script/reviewer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/reviewer.go b/script/reviewer.go index 60cd3cfc..5f088154 100644 --- a/script/reviewer.go +++ b/script/reviewer.go @@ -70,7 +70,7 @@ func (r *Reviewer) createComment(status Status) { } } else { title = "### ❌ Your application is invalid" - body = fmt.Sprintf("\n\n%s\n\nOur automated pre-checks have detected the following issues:\n\n%s", details, r.application.RenderProblems()) + body = fmt.Sprintf("\n\n%s\n\nOur automated pre-checks have detected the following problems:\n\n%s\n\nUpdate this issue to correct these problems and we’ll automatically re-evaluate your application.", details, r.application.RenderProblems()) } r.gitHub.CreateIssueComment(fmt.Sprintf("%s%s", title, body)) From 26f627a8132066430d15c71b2c17fbfd06e64c9e Mon Sep 17 00:00:00 2001 From: Jody Heavener Date: Thu, 28 Mar 2024 11:01:37 -0700 Subject: [PATCH 10/10] Update logic and copy for closed/approved applications to include path to application file --- script/reviewer.go | 48 +++++++++------ .../valid-project-approved-closed.json | 60 +++++++++++++++++++ 2 files changed, 89 insertions(+), 19 deletions(-) create mode 100644 script/test-issues/valid-project-approved-closed.json diff --git a/script/reviewer.go b/script/reviewer.go index 5f088154..a47a1ebc 100644 --- a/script/reviewer.go +++ b/script/reviewer.go @@ -8,8 +8,7 @@ import ( type Status int const ( - Closed Status = iota - Approved + Approved Status = iota Reviewing Invalid New @@ -31,14 +30,14 @@ func (r *Reviewer) Review() { r.application.Parse(r.gitHub.Issue) status := r.getStatus() - r.updateLabels(status) - r.createComment(status) + isClosed := *r.gitHub.Issue.State == "closed" + + r.updateLabels(status, isClosed) + r.createComment(status, isClosed) } func (r *Reviewer) getStatus() Status { - if *r.gitHub.Issue.State == "closed" { - return Closed - } else if r.gitHub.IssueHasLabel(LabelStatusApproved) { + if r.gitHub.IssueHasLabel(LabelStatusApproved) { return Approved } else if r.gitHub.IssueHasLabel(LabelStatusReviewing) { return Reviewing @@ -49,35 +48,46 @@ func (r *Reviewer) getStatus() Status { } } -func (r *Reviewer) createComment(status Status) { +func (r *Reviewer) createComment(status Status, isClosed bool) { title := "" body := "" - details := fmt.Sprintf("
\nApplication data...\n\n```json\n%s\n```\n
", r.application.GetData()) - // TODO: replace FILE_NAME with Application.FileName once available - dataPath := fmt.Sprintf("https://github.com/1Password/1password-teams-open-source/blob/main/data/%s", "FILE_NAME") - if status == Closed { - body = "This application is closed and changes will not be reviewed. If you have any questions, contact us at [opensource@1password.com](mailto:opensource@1password.com)." + applicationData := fmt.Sprintf("
\nApplication data...\n\n```json\n%s\n```\n
", r.application.GetData()) + applicationFilePath := fmt.Sprintf("https://github.com/1Password/1password-teams-open-source/blob/main/data/%s", r.application.FileName()) + approvedBody := fmt.Sprintf("This application has already been approved and changes will not be reviewed. If you would like to modify the details of your application, submit a pull request against the stored [application data](%s). If you have any questions, contact us at [opensource@1password.com](mailto:opensource@1password.com).", applicationFilePath) + closedBody := "This application is closed and changes will not be reviewed. If you have any questions, contact us at [opensource@1password.com](mailto:opensource@1password.com)." + + // If the issue is closed, let the user know that they can't make changes. + // If the issue was closed because it got approved, let them know how they can + // modify their application details after the fact. + if isClosed { + if status == Approved { + body = approvedBody + } else { + body = closedBody + } + // This scanerio should never occur, as an approved issue should + // immediately be closed, but let's cover all bases. } else if status == Approved { - body = fmt.Sprintf("This application has already been approved and changes will not be reviewed. If you would like to modify the details of your application, submit a pull request against the stored [application data](%s). If you have any questions, contact us at [opensource@1password.com](mailto:opensource@1password.com).", dataPath) + body = approvedBody } else if r.application.IsValid() { if status == Reviewing { title = "### 👍 Application still valid" - body = fmt.Sprintf("\n\n%s\n\nWe’ve run our automated pre-checks and your updated application is still valid.", details) + body = fmt.Sprintf("\n\n%s\n\nWe’ve run our automated pre-checks and your updated application is still valid.", applicationData) } else { title = "### ✅ Your application is valid" - body = fmt.Sprintf("\n\n%s\n\nThanks for applying! Our automated pre-checks have determined your application is valid. Next step: our team will review your application and may have follow-up questions. You can still make changes to your application and it’ll be re-evaluated.", details) + body = fmt.Sprintf("\n\n%s\n\nThanks for applying! Our automated pre-checks have determined your application is valid. Next step: our team will review your application and may have follow-up questions. You can still make changes to your application and it’ll be re-evaluated.", applicationData) } } else { title = "### ❌ Your application is invalid" - body = fmt.Sprintf("\n\n%s\n\nOur automated pre-checks have detected the following problems:\n\n%s\n\nUpdate this issue to correct these problems and we’ll automatically re-evaluate your application.", details, r.application.RenderProblems()) + body = fmt.Sprintf("\n\n%s\n\nOur automated pre-checks have detected the following problems:\n\n%s\n\nUpdate this issue to correct these problems and we’ll automatically re-evaluate your application.", applicationData, r.application.RenderProblems()) } r.gitHub.CreateIssueComment(fmt.Sprintf("%s%s", title, body)) } -func (r *Reviewer) updateLabels(status Status) { - if status == Approved || status == Closed { +func (r *Reviewer) updateLabels(status Status, isClosed bool) { + if status == Approved || isClosed { return } diff --git a/script/test-issues/valid-project-approved-closed.json b/script/test-issues/valid-project-approved-closed.json new file mode 100644 index 00000000..e8af7923 --- /dev/null +++ b/script/test-issues/valid-project-approved-closed.json @@ -0,0 +1,60 @@ +{ + "id": 1801650328, + "number": 6, + "state": "closed", + "locked": false, + "title": "Application for TestDB", + "body": "### Account URL\n\ntestdb.1password.com\n\n### Non-commercial confirmation\n\n- [X] No, this account won't be used for commercial activity\n\n### Team application\n\n- [ ] Yes, this application is for a team\n\n### Event application\n\n- [ ] Yes, this application is for an event\n\n### Project name\n\nTestDB\n\n### Short description\n\nTestDB is a free and open source, community-based forum software project.\n\n### Number of team members/core contributors\n\n1\n\n### Homepage URL\n\nhttps://github.com/wendyappleed/test-db\n\n### Repository URL\n\nhttps://github.com/wendyappleed/test-db\n\n### License type\n\nMIT\n\n### License URL\n\nhttps://github.com/wendyappleed/test-db/blob/main/LICENSE.md\n\n### Age confirmation\n\n- [X] Yes, this project is at least 30 days old\n\n### Name\n\nWendy Appleseed\n\n### Email\n\nwendyappleseed@example.com\n\n### Project role\n\nCore Maintainer\n\n### Profile or website\n\nhttps://github.com/wendyappleseed/\n\n### Can we contact you?\n\n- [X] Yes, you may contact me\n\n### Additional comments\n\nThank you!", + "user": { + "login": "wendyappleseed", + "id": 38230737, + "node_id": "MDQ6VXNlcjYzOTIwNDk=", + "avatar_url": "https://avatars.githubusercontent.com/u/38230737?v=4", + "html_url": "https://github.com/wendyappleseed", + "gravatar_id": "", + "type": "User", + "site_admin": false, + "url": "https://api.github.com/users/wendyappleseed", + "events_url": "https://api.github.com/users/wendyappleseed/events{/privacy}", + "following_url": "https://api.github.com/users/wendyappleseed/following{/other_user}", + "followers_url": "https://api.github.com/users/wendyappleseed/followers", + "gists_url": "https://api.github.com/users/wendyappleseed/gists{/gist_id}", + "organizations_url": "https://api.github.com/users/wendyappleseed/orgs", + "received_events_url": "https://api.github.com/users/wendyappleseed/received_events", + "repos_url": "https://api.github.com/users/wendyappleseed/repos", + "starred_url": "https://api.github.com/users/wendyappleseed/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/wendyappleseed/subscriptions" + }, + "labels": [ + { + "id": 5728067083, + "url": "https://api.github.com/repos/1Password/1password-teams-open-source/labels/status:%20approved", + "name": "status: approved", + "color": "0052CC", + "description": "The application has been approved", + "default": false, + "node_id": "LA_kwDOJ6JE6M8AAAABVWteCw" + } + ], + "comments": 11, + "closed_at": "2023-07-13T05:03:51Z", + "created_at": "2023-07-12T19:49:35Z", + "updated_at": "2023-07-13T05:03:51Z", + "url": "https://api.github.com/repos/1Password/1password-teams-open-source/issues/6", + "html_url": "https://github.com/wendyappleseed/1password-teams-open-source/issues/6", + "comments_url": "https://api.github.com/repos/1Password/1password-teams-open-source/issues/6/comments", + "events_url": "https://api.github.com/repos/1Password/1password-teams-open-source/issues/6/events", + "labels_url": "https://api.github.com/repos/1Password/1password-teams-open-source/issues/6/labels{/name}", + "repository_url": "https://api.github.com/repos/1Password/1password-teams-open-source", + "reactions": { + "total_count": 0, + "+1": 0, + "-1": 0, + "laugh": 0, + "confused": 0, + "heart": 0, + "hooray": 0, + "url": "https://api.github.com/repos/1Password/1password-teams-open-source/issues/6/reactions" + }, + "node_id": "I_kwDOJ6JE6M5rYwCY" +}