Skip to content

Conversation

AakashSuresh2003
Copy link

@AakashSuresh2003 AakashSuresh2003 commented Jun 24, 2025

…e test targeting

  • Did you write/update appropriate tests
  • Release notes updated (if appropriate)
  • Appropriate logging output
  • Issue linked
  • Docs updated (or issue created)
  • New package licenses are added to ThirdPartyNotices.txt (if applicable)

Added validation to ensure the following fields reject URLs and throw appropriate errors, as they are expected to contain plain names only:

GithubSourceOrg

GithubTargetOrg

SourceRepo

TargetRepo

Enterprise

These validations prevent incorrect usage like passing "https://github.com/org" instead of just "org".

@Copilot Copilot AI review requested due to automatic review settings June 24, 2025 11:38
Copilot

This comment was marked as outdated.

Copy link

github-actions bot commented Jun 24, 2025

Unit Test Results

  1 files    1 suites   10m 23s ⏱️
917 tests 917 ✅ 0 💤 0 ❌
918 runs  918 ✅ 0 💤 0 ❌

Results for commit eb6d32f.

♻️ This comment has been updated with latest results.

@AakashSuresh2003 AakashSuresh2003 requested a review from Copilot June 24, 2025 13:35
Copilot

This comment was marked as outdated.

@AakashSuresh2003 AakashSuresh2003 requested a review from Copilot June 24, 2025 14:03
Copilot

This comment was marked as outdated.

@AakashSuresh2003 AakashSuresh2003 requested a review from Copilot July 6, 2025 21:26
Copy link
Contributor

@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 adds validation to several command argument classes to reject full URL inputs for organization, repository, and enterprise name parameters, throwing user-friendly errors when URLs are provided. It also includes new unit tests to verify these validations.

  • Introduced ValidateNamesAreNotUrls in MigrateRepoCommandArgs and inline URL checks in MigrateOrgCommandArgs and GenerateScriptCommandArgs.
  • Updated existing Validate methods to call the new URL validation logic.
  • Added corresponding tests in three test files to ensure URL inputs throw the expected exceptions.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/gei/Commands/MigrateRepo/MigrateRepoCommandArgs.cs Added ValidateNamesAreNotUrls and call in Validate()
src/gei/Commands/MigrateOrg/MigrateOrgCommandArgs.cs Added URL checks for source org, target org, and enterprise
src/gei/Commands/GenerateScript/GenerateScriptCommandArgs.cs Added URL checks for source and target org fields
src/OctoshiftCLI.Tests/gei/Commands/MigrateRepo/MigrateRepoCommandArgsTests.cs Added tests for URL inputs in org and repo fields
src/OctoshiftCLI.Tests/gei/Commands/MigrateOrg/MigrateOrgCommandArgsTests.cs Added tests for URL inputs in org and enterprise fields
src/OctoshiftCLI.Tests/gei/Commands/GenerateScript/GenerateScriptCommandArgsTests.cs Added tests for URL inputs in org fields

@AakashSuresh2003 AakashSuresh2003 force-pushed the fix-url-input-error-1180 branch from 4b76367 to aa26cbf Compare August 19, 2025 08:03
@AakashSuresh2003 AakashSuresh2003 marked this pull request as draft September 9, 2025 06:23
@AakashSuresh2003 AakashSuresh2003 marked this pull request as ready for review September 9, 2025 06:24
Copy link

github-actions bot commented Sep 9, 2025

Code Coverage

Package Line Rate Branch Rate Complexity Health
gei 81% 72% 610
bbs2gh 83% 77% 653
ado2gh 83% 77% 613
Octoshift 87% 77% 1503
Summary 85% (7345 / 8661) 76% (1760 / 2312) 3379

@AakashSuresh2003
Copy link
Author

Hi @dylan-smith could you please review my request

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.

3 participants