Bitbucket Server Provider Fixes #928

MarkRx · 2024-05-30T13:07:18Z

User description

Seems this code hasn't been maintained. Contains the following fixes to minimally get it up and running:

Fix missing import
Fix missing attribute error when PR description is empty
Fix diff using incorrect to/from
Fix PR id retrieval
Fix some errors not being propagated
Fix description publishing not using correct url

Tested with BB Datacenter ("server" rebranded) v8.9.9

PR Type

Bug fix

Description

This PR includes several bug fixes and improvements for the Bitbucket Server provider:

Added missing import for quote_plus.
Introduced get_pr_id method to retrieve PR ID.
Corrected base and head SHA retrieval for diffs.
Ensured errors are raised for failed inline comment publishing.
Fixed URL generation for relevant line links.
Improved handling of inline comments, including multi-line and single-line comments.
Added check for PR description attribute existence.
Updated publish_description to use Bitbucket client and include reviewers.

Changes walkthrough 📝

Relevant files

Bug fix

bitbucket_server_provider.py `Multiple bug fixes and improvements for Bitbucket Server provider.` pr_agent/git_providers/bitbucket_server_provider.py Added missing import for `quote_plus`. Introduced `get_pr_id` method to retrieve PR ID. Corrected base and head SHA retrieval for diffs. Ensured errors are raised for failed inline comment publishing. Fixed URL generation for relevant line links. Improved handling of inline comments, including multi-line and single-line comments. Added check for PR description attribute existence. Updated `publish_description` to use Bitbucket client and include reviewers.	+29/-20

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

qodo-merge-pro · 2024-05-30T13:07:46Z

PR Review 🔍

⏱️ Estimated effort to review [1-5]	3, because the PR involves multiple bug fixes and improvements across various methods in a single file. The changes are moderate in number and complexity, requiring a thorough review to ensure that each fix is correctly implemented and does not introduce new issues.
🏅 Score	85
🧪 Relevant tests	No
⚡ Possible issues	Possible Bug: The method `get_pr_description_full` uses `hasattr(object, "description")` which should likely be `hasattr(self.pr, "description")`. Using `object` is incorrect and will not check the attribute on the `pr` instance.
⚡ Possible issues	Error Handling: The method `publish_inline_comments` logs an error but does not halt execution or handle the error beyond logging. This might lead to silent failures in inline comment publishing.
🔒 Security concerns	No

MarkRx · 2024-05-30T13:08:10Z

pr_agent/git_providers/bitbucket_server_provider.py

@@ -242,7 +239,7 @@ def publish_inline_comment(self, comment: str, from_line: int, file: str):
        }

        response = requests.post(url=self._get_pr_comments_url(""), json=payload, headers=self.headers)
-        return response
+        response.raise_for_status()


return isn't used and is inconsistent with other providers so I removed it

MarkRx · 2024-05-30T13:08:20Z

pr_agent/git_providers/bitbucket_server_provider.py

+            "reviewers": self.pr.reviewers # needs to be sent otherwise gets wiped
+        }
+
+        self.bitbucket_client.update_pull_request(self.workspace_slug, self.repo_slug, str(self.pr_num), payload)


return isn't used and is inconsistent with other providers so I removed it

qodo-merge-pro · 2024-05-30T13:08:26Z

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Correct the attribute check to use the `self.pr` object instead of the `object` class The `get_pr_description_full` method should check if the `self.pr` object has the `description` attribute, not the `object` class. pr_agent/git_providers/bitbucket_server_provider.py [286-289] -if hasattr(object, "description"): +if hasattr(self.pr, "description"): return self.pr.description else: return None Suggestion importance[1-10]: 10 Why: The suggestion correctly identifies a bug where the wrong object was checked for the attribute, which could lead to incorrect behavior. Fixing this is crucial for the method's functionality.	10
Best practice	Add exception handling to the `publish_description` method to ensure robustness The `publish_description` method should handle potential exceptions when updating the pull request to ensure robustness. pr_agent/git_providers/bitbucket_server_provider.py [352] -self.bitbucket_client.update_pull_request(self.workspace_slug, self.repo_slug, str(self.pr_num), payload) +try: + self.bitbucket_client.update_pull_request(self.workspace_slug, self.repo_slug, str(self.pr_num), payload) +except Exception as e: + get_logger().error(f"Failed to update pull request: {e}") Suggestion importance[1-10]: 8 Why: Adding exception handling is important for robustness, especially in network operations which are prone to failures. This suggestion enhances the reliability of the method.	8
Possible issue	Ensure `self.pr_url` is not `None` before generating the link to avoid potential errors In the `generate_link_to_relevant_line_number` method, handle the case where `self.pr_url` might be `None` to avoid potential errors. pr_agent/git_providers/bitbucket_server_provider.py [255-257] -if absolute_position != -1 and self.pr_url: - link = f"{self.pr_url}/diff#{quote_plus(relevant_file)}?t={absolute_position}" - return link +if absolute_position != -1: + if self.pr_url: + link = f"{self.pr_url}/diff#{quote_plus(relevant_file)}?t={absolute_position}" + return link + else: + get_logger().error("PR URL is not set.") Suggestion importance[1-10]: 7 Why: This suggestion addresses a potential issue where `self.pr_url` could be `None`, leading to an error when constructing the link. It's a good preventive measure.	7
Enhancement	Log the comment details when an inline comment cannot be published to aid in debugging In the `publish_inline_comments` method, consider logging the comment details when an inline comment cannot be published to aid in debugging. pr_agent/git_providers/bitbucket_server_provider.py [274] -get_logger().error(f"Could not publish inline comment {comment}") +get_logger().error(f"Could not publish inline comment: {comment}") Suggestion importance[1-10]: 6 Why: Enhancing the error message provides better context for debugging, which is helpful but not critical. The improvement is minor but useful for maintenance.	6

mrT23 · 2024-05-30T13:35:03Z

thanks @MarkRx
Bitbucket server support is a contribution from the community, and we rely on the community to maintain it.

have you reviewed the code suggestions ?
This seems like a bug, for example:

Correct the attribute check to use the self.pr object instead of the object class

The get_pr_description_full method should check if the self.pr object has the description
attribute, not the object class.

pr_agent/git_providers/bitbucket_server_provider.py [286-289]
-if hasattr(object, "description"):
+if hasattr(self.pr, "description"):
     return self.pr.description
 else:
     return None
Suggestion importance[1-10]: 10

Why: The suggestion correctly identifies a bug where the wrong object was checked for the attribute, which could lead to incorrect behavior. Fixing this is crucial for the method's functionality.

…eration, and pr review not working

mrT23 · 2024-05-30T18:13:46Z

@MarkRx
once you will fix the code suggestion, we can merge

qodo-merge-pro bot added Bug fix Review effort [1-5]: 3 labels May 30, 2024

MarkRx commented May 30, 2024

View reviewed changes

MarkRx force-pushed the bugfix/bb-server-fixes branch from 8b8db5b to 0f6d505 Compare May 30, 2024 14:00

BB Server fixes. Fix ID not being retrieved, inline comments, url gen…

ee90f38

…eration, and pr review not working

MarkRx force-pushed the bugfix/bb-server-fixes branch from 0f6d505 to ee90f38 Compare May 30, 2024 14:06

Improve bb server error logging

bd2f2b3

mrT23 approved these changes May 31, 2024

View reviewed changes

mrT23 merged commit ea9d410 into qodo-ai:main May 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Bitbucket Server Provider Fixes #928

Bitbucket Server Provider Fixes #928

Uh oh!

MarkRx commented May 30, 2024 •

edited by qodo-merge-pro bot

Loading

Uh oh!

qodo-merge-pro bot commented May 30, 2024

Uh oh!

MarkRx May 30, 2024

Uh oh!

MarkRx May 30, 2024

Uh oh!

qodo-merge-pro bot commented May 30, 2024

Uh oh!

mrT23 commented May 30, 2024 •

edited

Loading

Uh oh!

mrT23 commented May 30, 2024

Uh oh!

Uh oh!

Bitbucket Server Provider Fixes #928

Bitbucket Server Provider Fixes #928

Uh oh!

Conversation

MarkRx commented May 30, 2024 • edited by qodo-merge-pro bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

PR Type

Description

Changes walkthrough 📝

Uh oh!

qodo-merge-pro bot commented May 30, 2024

PR Review 🔍

Uh oh!

MarkRx May 30, 2024

Choose a reason for hiding this comment

Uh oh!

MarkRx May 30, 2024

Choose a reason for hiding this comment

Uh oh!

qodo-merge-pro bot commented May 30, 2024

PR Code Suggestions ✨

Uh oh!

mrT23 commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrT23 commented May 30, 2024

Uh oh!

Uh oh!

MarkRx commented May 30, 2024 •

edited by qodo-merge-pro bot

Loading

mrT23 commented May 30, 2024 •

edited

Loading