Fix project status unmarshaling

[williammartin]

Description

This PR fixes #8358

In our previous attempt to fix this, we tried some tricks in an attempt to unmarshal a fragment specified with a graphql struct tag into a single field with a json struct tag. As it turns out, shurcool, our graphql library does not respect json tags at all. In its custom JSON decoder, it expects that fragments are unmarshalled into structs (which is pretty reasonable) so it was failing to discover the field we wanted the data to be unmarshaled into, erroring like so:

struct field for "optionId" doesn't exist in any of 3 places to unmarshal

This PR returns the top level api Project structs to their old form, with only json struct tags, so that our own graphql client can decode GQL responses directly. We then take the shurcool graphql struct tags and throw them onto private structs inside the functions that use the shurcool client. We then transform those responses into the top level Project types.

We also add a bunch of tests here for good measure.

[andyfeller]

Only a super minor nit as everything else seems sound. I'm going to build this locally and do some testing for live review ...

[andyfeller]

Nothing blocking this review, but I can't help but feel like this use case should be documented as part of the shurcool repository. Just making a note for myself as I'm motivated to do it.

[williammartin]

I think it's reflected in https://github.com/shurcooL/graphql#inline-fragments:

type (
	DroidFragment struct {
		PrimaryFunction graphql.String
	}
	HumanFragment struct {
		Height graphql.Float
	}
)

var q struct {
	Hero struct {
		Name          graphql.String
		DroidFragment `graphql:"... on Droid"`
		HumanFragment `graphql:"... on Human"`
	} `graphql:"hero(episode: \"JEDI\")"`
}

[williammartin]

But I guess there's some extra craziness?

[andyfeller]

One problem found by @mattruggio is the difference between pr view and pr list handling of --json projectItems:

andyfeller@Andrews-MacBook-Pro:tinyfists/fork-testing ‹main›$ ~/Documents/workspace/cli/cli/main pr view 1 --json projectItems
{
  "projectItems": [
    {
      "status": {
        "optionId": "4cc61d42",
        "name": "In review"
      },
      "title": "@andyfeller's untitled project"
    }
  ]
}

whereas pr list isn't populating projectItems:

andyfeller@Andrews-MacBook-Pro:tinyfists/fork-testing ‹main›$ ~/Documents/workspace/cli/cli/main pr list --json id,number,projectItems
[
  {
    "id": "PR_kwDOKym29s5gcooL",
    "number": 2,
    "projectItems": []
  },
  {
    "id": "PR_kwDOKym29s5gcdEp",
    "number": 1,
    "projectItems": [
      {
        "status": {
          "optionId": "",
          "name": ""
        },
        "title": "@andyfeller's untitled project"
      }
    ]
  }
]

I know you're already aware of this, @williammartin, but adding the comment for posterity from internal discussion.

[mattruggio]

The change in this PR does not look to cause any regressions around previous work. Since the initial change did cause a bug in another area I wanted to aggregate some of the on-going work to provide a quick sanity check:

Initial change which introduced the new status field

Example(s):

gh issue list --repo mattruggio/kitkat --json projectItems
gh pr list --repo mattruggio/kitkat --json projectItems

The initial change did not account for an additional execution path which introduced a bug against editing PRs

Example(s):

gh issue edit 6 --repo mattruggio/kitkat --add-label "bug"
gh pr edit 2 --repo mattruggio/kitkat --add-label "enhancement"

Miscellaneous Calls

Example(s):

gh pr view 2 --repo mattruggio/kitkat --json projectItems
gh issue view 6 --repo mattruggio/kitkat --json projectItems

This PR: editing a PR without arguments

Example(s):

gh pr edit https://github.com/mattruggio/kitkat/pull/2

---

Notes:

• It is possible I am missing something, please let me know and I can add/re-test
• Some of the calls above are based on my own public repo so the calls may need to be edited to suit other's executions

[samcoe]

Do we want to change this to be ProjectV2ItemProject now?

[williammartin]

It doesn't seem to matter since it's private and we never construct it.

[samcoe]

Does it make the loop below through query.Repository.Issue.ProjectItems.Nodes slightly easier though? Or rather less code.

[williammartin]

Mmm, I wouldn't do it for less code but maybe because then we'd know if one changed the other should too. I dunno, it seems tenuous. There's also a nice advantage to treating these things separately since if we decide we need more shurcool magic in one we can avoid it in the other.

I'd favour treating them separately and adding a comment later (not blocking this PR cause I don't have time right now)

[andyfeller]

There's also a nice advantage to treating these things separately since if we decide we need more shurcool magic in one we can avoid it in the other.

This has been one of the struggles I've experienced using shurcool library, whether the query structure is what I pass back to code or having separate structures that don't couple logic to graphql.

[williammartin]

I would make the argument that data transfer structures should be separate from the core domain types. This applies both on ingress (Rest, GraphQL) and egress (Rest, GraphQL, JSON Export!).

Rust makes this kind of shuffling between domain types super simple but it's kind of gross in Go. You really need something like https://github.com/josebalius/exhauststruct to lint for fields that aren't copied between types correctly.

[samcoe]

Do we want to change this to be ProjectV2ItemProject now?

[williammartin]

It doesn't seem to matter since it's private and we never construct it.

[andyfeller]

Reviewing the code changes and doing manual local end-to-end testing, I think these changes are good to go 👍

andyfeller@Andrews-MacBook-Pro:tinyfists/fork-testing ‹andyfeller/main›$ ~/Documents/workspace/cli/cli/main pr list --json id,number,projectItems
[
  {
    "id": "PR_kwDOKym29s5gcooL",
    "number": 2,
    "projectItems": []
  },
  {
    "id": "PR_kwDOKym29s5gcdEp",
    "number": 1,
    "projectItems": [
      {
        "status": {
          "optionId": "4cc61d42",
          "name": "In review"
        },
        "title": "@andyfeller's untitled project"
      }
    ]
  }
]
andyfeller@Andrews-MacBook-Pro:tinyfists/fork-testing ‹andyfeller/main›$ ~/Documents/workspace/cli/cli/main pr view 1 --json projectItems        
{
  "projectItems": [
    {
      "status": {
        "optionId": "4cc61d42",
        "name": "In review"
      },
      "title": "@andyfeller's untitled project"
    }
  ]
}

[Doni108doni108]

****