Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions jf_agent/git/bitbucket_cloud_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@
for branch in get_branches_for_standardized_repo(repo, included_branches):
for j, commit in enumerate(
tqdm(
self.client.get_commits(repo.project.id, repo.id, branch),
self.client.get_commits(repo.project.id, _repo_slug(repo), branch),
desc=f'downloading commits for {repo.name} on branch {branch}',
unit='commits',
),
Expand All @@ -195,7 +195,7 @@
@diagnostics.capture_timing()
@logging_helper.log_entry_exit(logger)
def get_pull_requests(
self, standardized_repos: List[StandardizedRepository], server_git_instance_info,

Check warning on line 198 in jf_agent/git/bitbucket_cloud_adapter.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Add a type hint to this function parameter.

See more on https://sonarcloud.io/project/issues?id=Jellyfish-AI_jf_agent&issues=AZ3a6sp_Dz1M8EzFxFc9&open=AZ3a6sp_Dz1M8EzFxFc9&pullRequest=450
) -> List[StandardizedPullRequest]:
logger.info('downloading bitbucket prs...')
for i, repo in enumerate(
Expand All @@ -207,7 +207,7 @@
server_git_instance_info, repo.project.login, repo.id, 'prs'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Checking here, should this use the slug as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll double check, but I believe this and the other line can both be kept as UUIDs. The goal is to keep the UUID usage internally, but ensure API requests are all made using the slug.

)

api_prs = self.client.get_pullrequests(repo.project.id, repo.id)
api_prs = self.client.get_pullrequests(repo.project.id, _repo_slug(repo))

if not api_prs:
logger.info(f'no prs found for repo {repo.id}. Skipping... ')
Expand Down Expand Up @@ -265,12 +265,17 @@
@diagnostics.capture_timing()
@logging_helper.log_entry_exit(logger)
def get_branches(self, project, api_repo) -> List[StandardizedBranch]:
repo_slug = api_repo['full_name'].split('/')[-1]
return [
_standardize_branch(api_branch, self.config.git_redact_names_and_urls)
for api_branch in self.client.get_branches(project.id, api_repo['uuid'])
for api_branch in self.client.get_branches(project.id, repo_slug)
]


def _repo_slug(repo: StandardizedRepository) -> str:

Check warning on line 275 in jf_agent/git/bitbucket_cloud_adapter.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Add a docstring to this function.

See more on https://sonarcloud.io/project/issues?id=Jellyfish-AI_jf_agent&issues=AZ3bCeenRjRdAipT2cc5&open=AZ3bCeenRjRdAipT2cc5&pullRequest=450
return repo.full_name.split('/')[-1]


def _standardize_project(project_name, redact_names_and_urls):
return StandardizedProject(id=project_name, login=project_name, name=project_name, url=None)

Expand Down Expand Up @@ -378,12 +383,14 @@


def _standardize_pr(
client, repo, api_pr, strip_text_content: bool, redact_names_and_urls: bool,

Check warning on line 386 in jf_agent/git/bitbucket_cloud_adapter.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Add a type hint to this function parameter.

See more on https://sonarcloud.io/project/issues?id=Jellyfish-AI_jf_agent&issues=AZ3a6sp_Dz1M8EzFxFc-&open=AZ3a6sp_Dz1M8EzFxFc-&pullRequest=450

Check warning on line 386 in jf_agent/git/bitbucket_cloud_adapter.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Add a type hint to this function parameter.

See more on https://sonarcloud.io/project/issues?id=Jellyfish-AI_jf_agent&issues=AZ3a6sp_Dz1M8EzFxFdA&open=AZ3a6sp_Dz1M8EzFxFdA&pullRequest=450

Check warning on line 386 in jf_agent/git/bitbucket_cloud_adapter.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Add a type hint to this function parameter.

See more on https://sonarcloud.io/project/issues?id=Jellyfish-AI_jf_agent&issues=AZ3a6sp_Dz1M8EzFxFc_&open=AZ3a6sp_Dz1M8EzFxFc_&pullRequest=450
):
repo_slug = _repo_slug(repo)

# Process the PR's diff to get additions, deletions, changed_files
additions, deletions, changed_files = None, None, None
try:
diff_str = client.pr_diff(repo.project.id, repo.id, api_pr['id'])
diff_str = client.pr_diff(repo.project.id, repo_slug, api_pr['id'])
additions, deletions, changed_files = _calculate_diff_counts(diff_str)
if additions is None:
logging_helper.log_standard_error(
Expand Down Expand Up @@ -419,7 +426,7 @@
body=sanitize_text(c['content']['raw'], strip_text_content),
created_at=parser.parse(c['created_on']),
)
for c in client.pr_comments(repo.project.id, repo.id, api_pr['id'])
for c in client.pr_comments(repo.project.id, repo_slug, api_pr['id'])
]

# Crawl activity for approvals, merge and closed dates
Expand All @@ -428,7 +435,7 @@
merged_by = None
closed_date = None
try:
activity = list(client.pr_activity(repo.project.id, repo.id, api_pr['id']))
activity = list(client.pr_activity(repo.project.id, repo_slug, api_pr['id']))
approvals = [
StandardizedPullRequestReview(
user=_standardize_user(approval['user']),
Expand Down Expand Up @@ -470,7 +477,7 @@
strip_text_content,
redact_names_and_urls,
)
for c in client.pr_commits(repo.project.id, repo.id, api_pr['id'])
for c in client.pr_commits(repo.project.id, repo_slug, api_pr['id'])
]
merge_commit = None
if (
Expand All @@ -480,7 +487,7 @@
and api_pr['merge_commit'].get('hash')
):
api_merge_commit = client.get_commit(
repo.project.id, api_pr['source']['repository']['uuid'], api_pr['merge_commit']['hash']
repo.project.id, repo_slug, api_pr['merge_commit']['hash']
)
merge_commit = _standardize_commit(
api_merge_commit,
Expand Down
69 changes: 68 additions & 1 deletion tests/test_bitbucket_cloud_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
resulting_repo = resulting_repos[0]
input_repo = test_repos[0]
self.assertEqual(
resulting_repo.id, input_repo['uuid'], "Resulting repo id does not match input"
resulting_repo.id, input_repo['uuid'], "Resulting repo id should be the UUID"
)
self.assertEqual(
resulting_repo.name, input_repo['name'], "Resulting repo name does not match input"
Expand Down Expand Up @@ -282,6 +282,73 @@
self.assertFalse(resulting_pr.is_closed)
self.assertFalse(resulting_pr.is_merged)

def test_get_prs_merge_commit_uses_destination_repo(self):

Check warning on line 285 in tests/test_bitbucket_cloud_adapter.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Add a return type hint to this function declaration.

See more on https://sonarcloud.io/project/issues?id=Jellyfish-AI_jf_agent&issues=AZ3fHAq_Pn-QGctd7qTt&open=AZ3fHAq_Pn-QGctd7qTt&pullRequest=450
# Arrange: a MERGED PR where source repo uuid differs from destination repo slug.
# This verifies the fix for the bug where get_commit was called with the source
# repo uuid instead of the destination repo id.
test_commits = _get_test_data('test_commits.json')
dest_slug = 'destination_repo_slug'
source_uuid = '{aaaaaaaa-0000-0000-0000-bbbbbbbbbbbb}'

merged_pr = {
'id': 99,
'title': 'Merged PR',
'description': '',
'state': 'MERGED',
'merge_commit': {'hash': 'abc123merge'},
'created_on': '2020-01-01T00:00:00+00:00',
'updated_on': '2020-01-02T00:00:00+00:00',
'author': {
'display_name': 'test_user',
'uuid': '{1cd06601-cd0e-4fce-be03-e9ac226978b7}',
'links': {'html': {'href': 'https://bitbucket.org/test_user/'}},
},
'source': {
'branch': {'name': 'feature'},
'repository': {
'full_name': 'some-fork/destination_repo_slug',
'name': 'fork_repo',
'type': 'repository',
'uuid': source_uuid,
'links': {'self': {'href': 'https://api.bitbucket.org/2.0/repositories/some-fork/fork'}},
},
},
'destination': {
'branch': {'name': 'master'},
'repository': {
'full_name': f'test_project/{dest_slug}',
'name': dest_slug,
'type': 'repository',
'uuid': '{cccccccc-0000-0000-0000-dddddddddddd}',
'links': {'self': {'href': f'https://api.bitbucket.org/2.0/repositories/test_project/{dest_slug}'}},
},
},
'links': {'html': {'href': 'https://bitbucket.org/test_project/pull-requests/99'}},
}

mock_standardized_repo = MagicMock()
mock_standardized_repo.id = '{cccccccc-0000-0000-0000-dddddddddddd}'
mock_standardized_repo.full_name = f'test_project/{dest_slug}'
mock_standardized_repo.project.id = 'test_project'
mock_standardized_repos = [mock_standardized_repo]

self.mock_client.get_pullrequests.return_value = [merged_pr]
self.mock_client.pr_diff.return_value = ""
self.mock_client.pr_comments.return_value = []
self.mock_client.pr_activity.return_value = []
self.mock_client.pr_commits.return_value = test_commits
self.mock_client.get_commit.return_value = test_commits[0]

test_git_instance_info = {'pull_from': '1900-07-23', 'repos_dict_v2': {}}

# Call the adapter method
list(self.adapter.get_pull_requests(mock_standardized_repos, test_git_instance_info))

# Assert: get_commit must be called with the destination repo slug, not the source uuid
self.mock_client.get_commit.assert_called_once_with(
'test_project', dest_slug, 'abc123merge'
)


def _get_test_data(file_name):
with open(f'{TEST_INPUT_FILE_PATH}{file_name}', 'r') as f:
Expand Down
Loading