Add azure devops PR related implementations

[CTY-git]

PR Checklist

• The commit message follows our guidelines: Code of conduct
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)
• Does this PR introduce a breaking change?
• Include PR in release notes?

PR Type

• Bugfix
• Feature
• Refactoring
• Build /CI
• Documentation
• Others

What is the current behavior?

Azure Devops can only create PRs

Issue Number: #999

What is the new behavior?

Azure Devops can create PRs and create comments.

Other information

[patched-admin]

The pull request review identifies several key areas of concern in the proposed code changes, including potential bugs, security vulnerabilities, adherence to coding standards, and redundant operations. It highlights a potential bug in the get_slug_and_id_from_url method due to its assumption of a specific URL format, leading to incorrect parsing. Security concerns are raised regarding the create_comment method's direct logging of exceptions without sanitization, while the handling of azuredevops_api_key lacks validation or encryption measures, potentially introducing security vulnerabilities. The review also questions the updated type adherence in methods like find_pr_by_id, potentially affecting polymorphism and interface usage, and the removal of input validation checks with required_keys which might lead to runtime errors. Redundant fetching operations in the texts method and limited exception handling in the test method are flagged for potential improvement. Unused imports such as urlparse are noted for removal, and the switch from the old Comment class requires thorough integration to prevent inconsistencies. Overall, the review emphasizes the need for comprehensive exception handling, secure API key management, and adherence to coding standards, including proper documentation. The changes to pyproject.toml and test files were found to adhere to coding standards without introducing new issues.

---

• File changed: patchwork/common/client/scm.py
  1. Potential Bug: In the get_slug_and_id_from_url method, the URL splitting logic assumes a specific format for the URL, which may not be consistent across different users' configurations or repositories. This could lead to incorrect parsing and erroneous outputs.

2. Security Concerns: In the create_comment method, exceptions are logged directly without any sanitization or classification. Sensitive information could be exposed if exceptions contain any sensitive data. It would be better to have more granular exception handling and logging mechanisms that can categorize exceptions appropriately and log messages without sensitive details.

3. Coding Standards: The method signatures for converting types (e.g., GitlabMergeRequest, GithubPullRequest) in find_pr_by_id, get_pr_by_url, etc., seem changed from a general PullRequestProtocol. Ensure that this updated type adherence aligns with your project's planned refactoring strategy, as it might have implications on polymorphism and interface usage elsewhere in your codebase.

4. Redundant Fetch: In texts method, the invocation of for remote in repo.remotes: remote.fetch() updates the state of the local repository with the remote refs but may not be necessary for constructing diffs from commits that are already locally available. Consider if this fetch is needed or if it should be made conditional.

5. Error Handling: In the method test, only a ValueError exception is caught, which might not be comprehensive enough for the potential exceptions that can be raised by self.project. Implementing broader exception handling can enhance robustness. This same issue is present in multiple methods throughout the codebase, and a consistent strategy should be applied for exception handling.

6. Unused Imports: There might be unused imports (e.g., urlparse and define) removed, ensure these were not needed elsewhere before removal.

7. Differing Comment Structures: The old Comment class was removed. Ensure the code using the related comment structures was entirely modified to suit the new way comments are handled to avoid runtime errors or logical inconsistencies.

• File changed: patchwork/steps/CreatePRComment/CreatePRComment.py
  1. Potential Bugs:
  • The code currently doesn't handle the case where neither github_api_key, gitlab_api_key, nor azuredevops_api_key is provided. While there's a ValueError thrown for missing GitHub or GitLab keys, Azure DevOps is not yet integrated within that error message. Update the message to mention azuredevops_api_key as well.

2. Security Vulnerabilities:

   • Ensure that the AzureDevopsClient class does not have any known vulnerabilities when handling API keys. API keys should be stored securely, and the sensitivity of this information should be considered when logging and error handling.

3. Coding Standards:

   • The new code appears to follow the original coding standards, as it maintains similar conditional checks for additional API keys and consistency with how different SCM clients are initialized. However, consider using a more standardized approach, such as configuration files or environment variables, for managing API keys securely.

Overall, ensure that error messages and logging do not expose API keys or sensitive input data, and consider improving the configurability of API key handling to adhere to best practices in security management.

• File changed: patchwork/steps/CreatePRComment/typed.py
  1. Security Vulnerability: Adding the azuredevops_api_key alongside gitlab_api_key and github_api_key without proper validation or encryption could introduce security vulnerabilities. Ensure that API keys are stored securely and validate them properly to prevent unauthorized access.

2. Coding Standards: It is not clear if the introduction of azuredevops_api_key adheres to the existing coding standards. There should be supporting documentation or comments explaining this addition and how it fits into existing logic and structure.

3. Potential Bugs: The logic change with 'or_op' now includes three keys. Ensure there is test coverage for scenarios involving combinations of these keys to prevent bugs related to order of precedence and unexpected null values.

• File changed: patchwork/steps/ReadPRDiffs/ReadPRDiffs.py
  1. Potential Bugs:
  • The check for required keys, particularly "pr_url", has been removed (required_keys attribute). This may lead to potential issues where the expected URL might be missing from the input, causing runtime errors.

2. Security Vulnerabilities:

• Ensure that the handling of SCM clients (Github, Gitlab, AzureDevops) keys is secure to prevent leakage of sensitive information. This code does not seem to implement new input validation or sanitation which could expose the process to injection-like vulnerabilities if user input is not properly validated elsewhere.

3. Adherence to Coding Standards:

• It seems like the new initialization condition for AzureDevopsClient follows the existing pattern for Github and Gitlab. However, the removal of required_keys validation deviates from established practices of input validation as initially present in the code. It might be beneficial to maintain such validation to ensure that all necessary inputs are present before proceeding.

• File changed: patchwork/steps/ReadPRDiffs/typed.py
  1. Potential Bugs:
  • Adding azuredevops_api_key alongside existing keys in or_op might result in logic issues if the function that uses these keys assumes only two possible options when resolving which API key to use. Ensure the function logic that processes or_op can handle more than two scenarios correctly.

2. Security Vulnerabilities:

   • Introducing another API key handling introduces potential security risks, especially concerning secure storage and proper access management. Ensure that azuredevops_api_key receives equivalent handling in terms of secure storage and access restrictions as the other API keys in the system.

3. Coding Standards:

   • Assuming the previous configuration for or_op was meant to maintain exactly two keys, introducing a third (azuredevops_api_key) changes the underlying assumptions of this logic. Check if the widening of options adheres to the intended design pattern or if additional logic is required to accommodate a third option properly.
   • Ensure that documentation and inline comments are updated to reflect the changes appropriately for maintainability and clarity, especially with the introduction of azuredevops_api_key.

• File changed: pyproject.toml
  The change in 'pyproject.toml' is an update of the version number for the package 'patchwork-cli' from '0.0.85' to '0.0.86'.

• There are no code modifications here that could introduce functional bugs or security vulnerabilities as these changes are limited to metadata.
• The update itself is in line with the original coding standards for versioning as it follows semantic versioning practices where a patch version is being incremented.

No issues found in this pull request according to the provided instruction.

• File changed: tests/common/test_app.py
  The given changes are modifications to a test file test_app.py, specifically to tests for list options. The addition of GenerateCodeUsageExample in the expected list output seems to be a simple addition and doesn't introduce any apparent bugs as it's a straightforward change in test assertions. Here is the review based on instructed points:

• Potential Bugs: The change itself does not introduce any immediate bugs as it purely alters expected output lists in test functions.

• Security Vulnerabilities: The changes are contained within test files and only alter expected strings, hence they do not introduce new security vulnerabilities.

• Coding Standards: The modification follows the existing pattern in the test function and thus adheres to the coding standards applied in other parts of the file.

No issues found related to bugs, security, or code standards in this PR.