feat: Add ListFineGrainedPersonalAccessTokenRequests for org

[AbhishekAg]

Implement support for listing requests to access organization resources via fine-grained personal access tokens (GET /orgs/{org}/personal-access-token-requests).

• Add FineGrainedPersonalAccessTokenRequests struct for request details
  (id, reason, owner, repository selection, permissions, timestamps, token info)
• Add ListFineGrainedPATRequestOptions with TokenID filter and embedded
  ListFineGrainedPATOptions (sort, direction, owner, repository, permission, last_used_before/after, pagination)
• Add OrganizationsService.ListFineGrainedPersonalAccessTokenRequests
• Add addListFineGrainedPATRequestOptions helper for owner[] and token_id[] query parameters

API docs: https://docs.github.com/en/rest/orgs/personal-access-tokens#list-requests-to-access-organization-resources-with-fine-grained-personal-access-tokens

Fixes: #4021

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.09%. Comparing base (c02c318) to head (8bb130d).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4022      +/-   ##
==========================================
+ Coverage   94.07%   94.09%   +0.02%     
==========================================
  Files         207      207              
  Lines       19163    19203      +40     
==========================================
+ Hits        18028    18070      +42     
+ Misses        936      935       -1     
+ Partials      199      198       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[AbhishekAg]

@gmlewis Can you please re-run the pipeline? linter error is due to 503

validating openapi_operations.yaml
unexpected status code: 503 Service Unavailable
failed validating openapi_operations.yaml
validating generated files

[gmlewis]

Thank you, @AbhishekAg!
Once you add omitempty to the pointer fields in FineGrainedPersonalAccessTokenRequests then I think we should be ready for a second LGTM+Approval from any other contributor to this repo before merging.

cc: @stevehipwell - @alexandear - @zyfy29 - @Not-Dhananjay-Mishra

[AbhishekAg]

Thank you, @AbhishekAg! Once you add omitempty to the pointer fields in FineGrainedPersonalAccessTokenRequests then I think we should be ready for a second LGTM+Approval from any other contributor to this repo before merging.

cc: @stevehipwell - @alexandear - @zyfy29 - @Not-Dhananjay-Mishra

Done.

[stevehipwell]

I may be missing something but to me the struct pattern doesn't look right/consistent?

[AbhishekAg]

Pipeline failure due to

--- FAIL: TestRepositoriesService_RemoveCollaborator_invalidUser (0.00s)
    testing.go:1712: race detected during execution of test
--- FAIL: TestRepositoriesService_UploadReleaseAssetFromRelease_AbsoluteTemplate (0.04s)
    testing.go:1712: race detected during execution of test

[gmlewis]

Pipeline failure due to

--- FAIL: TestRepositoriesService_RemoveCollaborator_invalidUser (0.00s)
    testing.go:1712: race detected during execution of test
--- FAIL: TestRepositoriesService_UploadReleaseAssetFromRelease_AbsoluteTemplate (0.04s)
    testing.go:1712: race detected during execution of test

Unfortunately, we appear to be having some flaky tests, which @alexandear also noticed and filed an issue: #3987.

I'm not sure at what point this started getting flaky, but it feels like it is pretty recent.

I reran the failed job.

[alexandear]

By the way, did you check ListFineGrainedPersonalAccessTokenRequests by calling it on a real organization instead of using mocked data?

[gmlewis]

If GitHub doesn’t send some fields, that’s a mistake on GitHub’s side.

Yes, and over the last 10+ years, we've seen that a fair amount... mostly in the early days (hence the stance in our README.md).
Granted that GitHub was not even providing response schemas in the early days.

OK, so your argument is that now that GitHub IS providing response schemas, and we are building a new endpoint based upon that schema, that we should initially attempt to honor it and fix bugs as they occur.

I totally understand that argument.

As a maintainer, we wish that everything could be 100% consistent and cut-and-dry and make perfect sense.

So my next thought is... now that GitHub response schemas are prolific, do we now go back and clean up every struct this repo generates for GitHub resources and make the fields match the schemas?!?

Are we opening up an enormous can of worms?

Thoughts?

[alexandear]

So my next thought is... now that GitHub response schemas are prolific, do we now go back and clean up every struct this repo generates for GitHub resources and make the fields match the schemas?!?

Are we opening up an enormous can of worms?

We can do this in the future, but we need to make sure it won't break anything. So we should limit these changes to endpoints that have integration tests. Or maybe we need to add more integration tests, but it's hard for some endpoints (e.g., enterprise or org-related).

[gmlewis]

OK, @AbhishekAg, let's run with the recommendations from @stevehipwell and @alexandear.
I apologize for steering you in the wrong direction.

[AbhishekAg]

I completed all the requested changes.

[stevehipwell]

LGTM

[gmlewis]

Thank you, @AbhishekAg and @stevehipwell!
I'm good with these changes.
LGTM.

@alexandear - have all your concerns been addressed before I merge?

[AbhishekAg]

@gmlewis can we please merge these changes if no more comments? All reviewers have approved the changes.