Support projects V2 in create issue and create PR commands #6043
Conversation
Adding methods similar to RepoProjects and OrganizationProjects to collect a list of RepoProjectV2. In constrast to projects old/v1, we cannot query projects by state. So, we filter out closed projects after fetching them.
With this change, project names are now mapped to a pair of ID lists. The first one contains IDs of old/v1 projects and the second contains the IDs of next/v2 projects. This allows us to exclude projectV2 IDs from IssueCreate and PullRequestCreate mutations and in the future, would allow us to include only projectV2 IDs in the addProjectV2ItemById mutation.
If creating the entity succeeded, then we add it by ID to all V2 projects. This will require triggering a mutation per project.
cliAutomation
added
the
external
| } | ||
|
|
||
| for _, p := range query.Organization.ProjectsV2.Nodes { | ||
| if !p.Closed { |
There was a problem hiding this comment.
💭 API does not allow to filter by state when querying, so filtering based on response
| } | ||
| projects = append(projects, orgProjects...) | ||
|
|
||
| return projects, nil | ||
| projectsV2, err := RepoProjectsV2(client, repo) |
There was a problem hiding this comment.
❓ Question from my side. This would mean that users would require to have read:project and read:org permissions which earlier was not the case. Is it fine to do it this way or should we look up these projects lazily only when there are potential project V2 references?
| @@ -671,3 +706,78 @@ func TestIssueCreate_AtMeAssignee(t *testing.T) { | |||
|
|
|||
| assert.Equal(t, "https://github.com/OWNER/REPO/issues/12\n", output.String()) | |||
| } | |||
|
|
|||
| func TestIssueCreate_projectsV2(t *testing.T) { | |||
There was a problem hiding this comment.
💭 I know that these are legacy tests, but I couldn't figure out how to create reasonable assertions with the other strategy. There is nothing in the console output to assert on, no errors, no link changes. We could potentially validate the content of the web page, as that should include the new project reference. But it requires much more setup and I decided to not go down that route.
There was a problem hiding this comment.
So, I mostly relied on manual testing: created a new project in my fork and was spamming it with issues and PRs
In constrast to regular field update, we track additions and deletions separately, since project items can be added to and remove from project, but not in a single mutation. To add item (i.e. issue or PR) to a project V2, we call AddProjectV2ItemById with the entity's ID and project's ID. Removing the item from a project is more tricky. The item known to the project does not correspond to an ID of an issue or PR. Instead, we firstly fetch all known item IDs that correspond to the issue or PR, and then remove those from all projects that are requested to be removed.
| @@ -36,6 +36,7 @@ type Issue struct { | |||
| Assignees Assignees | |||
| Labels Labels | |||
| ProjectCards ProjectCards | |||
| ProjectItems ProjectItems | |||
There was a problem hiding this comment.
💭 Pre-fetch references to items to avoid expensive lookup later on. We need the link because the entity ID does not match the ID of the corresponding item in Project V2. And project's item's ID is required to unlink item from the project.
| @@ -431,6 +438,27 @@ func mockRepoMetadata(_ *testing.T, reg *httpmock.Registry) { | |||
| "pageInfo": { "hasNextPage": false } | |||
| } } } } | |||
| `)) | |||
| reg.Register( | |||
There was a problem hiding this comment.
💭 Tests for editing are pretty shallow and I'd like to improve them, but I am a bit lost.. I am open for some advice on how to approach testing and set those up. Currently, I make sure existing tests are not broken and that the logic flow is correct and proper APIs are called. But testing the interactive mode more properly or different combinations would be nice
Prior to this commit, only projectV1 names were provided as hints when using interactive suvery. Now, projectV2 titles are also included in the proposal.
|
Original repo for this PR was unavailable, I created new PR with fixed tests and checked that it works. |
|
Closing in favor of #6735 |
Summary
Mainly following suggestions from #4547 (comment).
Creating Issue/PR
IssueCreate/PullRequestCreatemutation.AddProjectV2ItemById/DeleteProjectV2Itemmutation.Editing Issue/PR
3.1. all projects which are in the current selection, but to which the item does not belong, should be added.
3.2. all projects which are not in the current selection, but to which the item belongs, should be removed.
Notes: I am a bit lost with regards to the expected level of testing and could use some guidance here. I ensured that existing tests pass with expected reasons. I ensured that correct APIs are called when project V2 items are impacted. I also did some manual testing of (non-)interactive modes on my own repo and projects. I would like to have more automated testing, but am a bit lost in the current setup.
Fixes #4547