Add support for repository alphanumeric autolinks (Fixes #1270) #1314

douglascayers · 2022-10-02T19:19:31Z

Changes

Fixes Support Autolinks with alphanumeric or numeric IDs #1270
Add support for is_alphanumeric property of autolinks (available per Add alphanumeric param to AutolinkOptions google/go-github#2450)

Tests

github/resource_github_repository_autolink_reference_test.go
Adds tests when new is_alphanumeric property is not set (uses default) or is set to either true or false
Replace usage of "OOF" with "TEST" to be more explicit that these are test resources.
Update docs on usage

How to Test

Create a personal access token with the following scopes: repo, admin:org, and delete_repo.

export GITHUB_TOKEN="ghp_xxx"
export GITHUB_ORGANIZATION="your-org"
TF_ACC=1 go test -v -run TestAccGithubRepositoryAutolinkReference

Test Output

=== RUN   TestAccGithubRepositoryAutolinkReference
=== RUN   TestAccGithubRepositoryAutolinkReference/creates_repository_autolink_reference_without_error
=== RUN   TestAccGithubRepositoryAutolinkReference/creates_repository_autolink_reference_without_error/with_an_anonymous_account
    resource_github_repository_autolink_reference_test.go:94: anonymous account not supported for this operation
=== RUN   TestAccGithubRepositoryAutolinkReference/creates_repository_autolink_reference_without_error/with_an_individual_account
    provider_utils.go:56: GITHUB_TOKEN and GITHUB_OWNER environment variables should be set
    provider_utils.go:66: Skipping TestAccGithubRepositoryAutolinkReference/creates_repository_autolink_reference_without_error/with_an_individual_account which requires individual mode
=== RUN   TestAccGithubRepositoryAutolinkReference/creates_repository_autolink_reference_without_error/with_an_organization_account
=== RUN   TestAccGithubRepositoryAutolinkReference/imports_repository_autolink_reference_without_error
=== RUN   TestAccGithubRepositoryAutolinkReference/imports_repository_autolink_reference_without_error/with_an_anonymous_account
    resource_github_repository_autolink_reference_test.go:207: anonymous account not supported for this operation
=== RUN   TestAccGithubRepositoryAutolinkReference/imports_repository_autolink_reference_without_error/with_an_individual_account
    provider_utils.go:56: GITHUB_TOKEN and GITHUB_OWNER environment variables should be set
    provider_utils.go:66: Skipping TestAccGithubRepositoryAutolinkReference/imports_repository_autolink_reference_without_error/with_an_individual_account which requires individual mode
=== RUN   TestAccGithubRepositoryAutolinkReference/imports_repository_autolink_reference_without_error/with_an_organization_account
=== RUN   TestAccGithubRepositoryAutolinkReference/deletes_repository_autolink_reference_without_error
=== RUN   TestAccGithubRepositoryAutolinkReference/deletes_repository_autolink_reference_without_error/with_an_anonymous_account
    resource_github_repository_autolink_reference_test.go:250: anonymous account not supported for this operation
=== RUN   TestAccGithubRepositoryAutolinkReference/deletes_repository_autolink_reference_without_error/with_an_individual_account
    provider_utils.go:56: GITHUB_TOKEN and GITHUB_OWNER environment variables should be set
    provider_utils.go:66: Skipping TestAccGithubRepositoryAutolinkReference/deletes_repository_autolink_reference_without_error/with_an_individual_account which requires individual mode
=== RUN   TestAccGithubRepositoryAutolinkReference/deletes_repository_autolink_reference_without_error/with_an_organization_account
--- PASS: TestAccGithubRepositoryAutolinkReference (60.26s)
    --- PASS: TestAccGithubRepositoryAutolinkReference/creates_repository_autolink_reference_without_error (26.58s)
        --- SKIP: TestAccGithubRepositoryAutolinkReference/creates_repository_autolink_reference_without_error/with_an_anonymous_account (0.00s)
        --- SKIP: TestAccGithubRepositoryAutolinkReference/creates_repository_autolink_reference_without_error/with_an_individual_account (0.00s)
        --- PASS: TestAccGithubRepositoryAutolinkReference/creates_repository_autolink_reference_without_error/with_an_organization_account (26.58s)
    --- PASS: TestAccGithubRepositoryAutolinkReference/imports_repository_autolink_reference_without_error (30.42s)
        --- SKIP: TestAccGithubRepositoryAutolinkReference/imports_repository_autolink_reference_without_error/with_an_anonymous_account (0.00s)
        --- SKIP: TestAccGithubRepositoryAutolinkReference/imports_repository_autolink_reference_without_error/with_an_individual_account (0.00s)
        --- PASS: TestAccGithubRepositoryAutolinkReference/imports_repository_autolink_reference_without_error/with_an_organization_account (30.42s)
    --- PASS: TestAccGithubRepositoryAutolinkReference/deletes_repository_autolink_reference_without_error (3.26s)
        --- SKIP: TestAccGithubRepositoryAutolinkReference/deletes_repository_autolink_reference_without_error/with_an_anonymous_account (0.00s)
        --- SKIP: TestAccGithubRepositoryAutolinkReference/deletes_repository_autolink_reference_without_error/with_an_individual_account (0.00s)
        --- PASS: TestAccGithubRepositoryAutolinkReference/deletes_repository_autolink_reference_without_error/with_an_organization_account (3.26s)
PASS
ok      github.com/integrations/terraform-provider-github/v5/github     60.741s

Usage

Example configuration:

resource "github_repository_autolink_reference" "autolink" {
    repository          = github_repository.some-repo.name
    key_prefix          = "TICKET-"
    target_url_template = "https://example.com/TICKET-<num>"
    is_alphanumeric     = true
}

github/resource_github_repository_autolink_reference.go

douglascayers · 2022-10-02T20:15:38Z

github/resource_github_repository_autolink_reference.go

+				Type:        schema.TypeBool,
+				Optional:    true,
+				ForceNew:    true,
+				Default:     true,


is_alphanumeric is optional in the API and defaults to true

douglascayers · 2022-10-10T04:11:21Z

Hi @kfcampbell, this is ready for review, thanks!

kfcampbell · 2022-10-10T21:43:24Z

@douglascayers I see this makes the (breaking) change of switching between / and : as a delimiter between repo name and reference idea. I like this change, and I think we should be doing that. My concern is with the breaking nature of the change:

In my perfect world, it might be best to remove that functionality, add this feature, and then in a separate breaking change switch the delimiter. Do you think that's too much trouble? Alternately, I could cut v6.0.0 of the plugin and take this change wholesale.

douglascayers · 2022-10-10T22:31:00Z

@kfcampbell Good catch on the breaking change.

it might be best to remove that functionality, add this feature, and then in a separate breaking change switch the delimiter

Alternately, I could cut v6.0.0 of the plugin and take this change wholesale.

❓ Are there other PRs you'd like to merge into the 5.x.x line before cutting the 6.x.x version? If yes, I'm happy to split this PR into two to accommodate. If you're thinking you'll merge both PRs right after each other and end up on the 6.x.x version anyways without other changes in the 5.x.x line, perhaps we just go forward with this one PR? 🤔

kfcampbell · 2022-10-10T22:47:29Z

👍 I had hoped to merge a few other PRs to v5.x.x for small features and dependency updates and (again in my perfect world) do something to address #980 finally, though that item doesn't have to be a blocker for the delimiter change. If you could split it into two PRs, that would be awesome!

douglascayers · 2022-10-11T03:03:22Z

👍 I had hoped to merge a few other PRs to v5.x.x for small features and dependency updates and (again in my perfect world) do something to address #980 finally, though that item doesn't have to be a blocker for the delimiter change. If you could split it into two PRs, that would be awesome!

I've converted this to draft while I work on that, thanks

kfcampbell · 2022-10-11T16:16:25Z

Thank you!

…lphanumeric-autolinks

douglascayers · 2022-10-31T02:25:21Z

Hi @kfcampbell, this is ready for review, thanks!

Per your feedback, I've reverted the import syntax changes to avoid introducing a "breaking" change that would prompt a major version change.

If I revisit standardizing the import syntax for resources, I'll do that in an isolated PR for as many resources as I can update.

kfcampbell

@douglascayers awesome, thank you so much! I'll get this released shortly.

I love the idea of standardizing on the import syntax and would happily review future PRs in this direction.

… (#1314)" This reverts commit f7da074.

…#1270) (integrations#1314) * Add `is_alphanumeric` for repo autolinks * Test `is_alphanumeric` for repo autolinks * Use parseTwoPartID util func for consistent import ids * Update docs about `is_alphanumeric` and import format * revert import state parsing to avoid breaking change * revert import state parsing to avoid breaking change * revert import state parsing to avoid breaking change

douglascayers added 4 commits October 2, 2022 14:10

Add is_alphanumeric for repo autolinks

fbfc019

Test is_alphanumeric for repo autolinks

ba19188

Use parseTwoPartID util func for consistent import ids

a081869

Update docs about is_alphanumeric and import format

ee3e7ef

douglascayers commented Oct 2, 2022

View reviewed changes

github/resource_github_repository_autolink_reference.go Outdated Show resolved Hide resolved

douglascayers commented Oct 2, 2022

View reviewed changes

douglascayers changed the title ~~[resource_github_repository_autolink_reference] Add support for alphanumeric autolinks (Fixes #1270)~~ Add support for repository alphanumeric autolinks (Fixes #1270) Oct 3, 2022

douglascayers marked this pull request as draft October 11, 2022 03:02

douglascayers added 4 commits October 30, 2022 20:49

Merge remote-tracking branch 'upstream/main' into issue1270/support-a…

06342c4

…lphanumeric-autolinks

revert import state parsing to avoid breaking change

768aace

revert import state parsing to avoid breaking change

7bc1b85

revert import state parsing to avoid breaking change

41d1c3a

douglascayers marked this pull request as ready for review October 31, 2022 02:23

kfcampbell approved these changes Oct 31, 2022

View reviewed changes

kfcampbell merged commit f7da074 into integrations:main Oct 31, 2022

kfcampbell added a commit that referenced this pull request Oct 31, 2022

Revert "Add support for repository alphanumeric autolinks (Fixes #1270)…

11a3d74

… (#1314)" This reverts commit f7da074.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for repository alphanumeric autolinks (Fixes #1270) #1314

Add support for repository alphanumeric autolinks (Fixes #1270) #1314

douglascayers commented Oct 2, 2022 •

edited

Loading

douglascayers Oct 2, 2022

douglascayers commented Oct 10, 2022

kfcampbell commented Oct 10, 2022

douglascayers commented Oct 10, 2022

kfcampbell commented Oct 10, 2022

douglascayers commented Oct 11, 2022

kfcampbell commented Oct 11, 2022

douglascayers commented Oct 31, 2022

kfcampbell left a comment

Add support for repository alphanumeric autolinks (Fixes #1270) #1314

Add support for repository alphanumeric autolinks (Fixes #1270) #1314

Conversation

douglascayers commented Oct 2, 2022 • edited Loading

Changes

Tests

How to Test

Usage

douglascayers Oct 2, 2022

Choose a reason for hiding this comment

douglascayers commented Oct 10, 2022

kfcampbell commented Oct 10, 2022

douglascayers commented Oct 10, 2022

kfcampbell commented Oct 10, 2022

douglascayers commented Oct 11, 2022

kfcampbell commented Oct 11, 2022

douglascayers commented Oct 31, 2022

kfcampbell left a comment

Choose a reason for hiding this comment

douglascayers commented Oct 2, 2022 •

edited

Loading