Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a bug in fetching sub org repos by custom properties #792

Merged
merged 3 commits into from
Mar 22, 2025

Conversation

decyjphr
Copy link
Collaborator

There was a bug introduced during refactorings in the lib/settings.js file which resulted in sub-org repos not being fetched correctly for custom properties.

@Copilot Copilot bot review requested due to automatic review settings March 21, 2025 20:01
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request fixes an issue where sub-org repositories were not correctly fetched for custom properties and refactors the archive handling logic in the repo update process.

  • Refactored archive calls to separate unarchiving and archiving flows based on repo state.
  • Updated the custom properties query syntax from dot notation to colon notation.
  • Adjusted the handling of plugin results to ensure proper aggregation of output.
Comments suppressed due to low confidence (2)

lib/settings.js:337

  • The archive handling logic was refactored to separately address unarchiving and archiving. Please verify that executing archivePlugin.sync() twice, based on shouldUnarchive and shouldArchive conditions respectively, is the intended behavior to avoid any unintended side effects.
const archivePlugin = new Archive(this.nop, this.github, repo, repoConfig, this.log)

lib/settings.js:901

  • The custom properties query syntax has been updated from dot notation to colon notation. Confirm that this new query format aligns with the expected API parameters for fetching repository properties.
const query = `props.${name}:${value}`

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

@decyjphr decyjphr requested a review from Copilot March 21, 2025 20:05
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug in fetching sub-org repositories by custom properties. The key changes include:

  • Modifying the query string delimiter from a dot (.) to a colon (:) in the custom property query.
  • Replacing the call to this.github.repos.getCustomPropertiesValues with a new request endpoint.
  • Updating the pagination call accordingly.
Comments suppressed due to low confidence (2)

lib/settings.js:888

  • Ensure that the new query format using ':' as a delimiter is compatible with the API specifications. If the API expects a different delimiter or a different query structure, the query may not return the desired results.
const query = `props.${name}:${value}`

lib/settings.js:890

  • Consider using the previously defined organizationName variable instead of this.repo.owner for consistency in referencing the organization.
const options = this.github.request.endpoint((`/orgs/${this.repo.owner}/properties/values?repository_query=${encodedQuery}`))

Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more

@decyjphr
Copy link
Collaborator Author

decyjphr commented Mar 21, 2025

@PendaGTP I had to make a slight modification in the code for fetching repos by suborg properties because it was failing with the way it was fetching previously. Since the change involved the octokit query, I reset it back to the way it was where it was querying the organization for all repos matching the custom property value.

@decyjphr decyjphr requested a review from Copilot March 21, 2025 22:16
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request addresses a bug where sub-org repositories were not being fetched correctly due to an incorrect query format and API endpoint usage.

  • Updated the custom property query format from dot notation to colon notation.
  • Replaced the previous API call with a new endpoint that uses query parameters for fetching repository properties.
Comments suppressed due to low confidence (2)

lib/settings.js:888

  • The query format has been updated to use a colon instead of a period. Please confirm that this change is consistent with how the backend expects the query parameter format.
const query = `props.${name}:${value}`

lib/settings.js:890

  • The previous implementation included a 'per_page: 100' parameter to handle pagination. Verify that the new endpoint handles pagination appropriately or consider adding similar pagination parameters if needed.
const options = this.github.request.endpoint((`/orgs/${organizationName}/properties/values?repository_query=${encodedQuery}`))

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

@PendaGTP
Copy link
Contributor

@decyjphr I confirm there was an issue with searching repositories by custom property. GH docs indicate we should use props.PROPERTY:VALUE syntax instead of props.PROPERTY.VALUE. Sorry for missing this.

@decyjphr
Copy link
Collaborator Author

No worries I wanted to keep you in the loop. I didn't catch it either when I merged it. Thanks @PendaGTP

@decyjphr decyjphr merged commit d22c065 into main-enterprise Mar 22, 2025
5 checks passed
@decyjphr decyjphr deleted the fix-suborgs-by-props branch March 22, 2025 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants