Skip to content

Bug Jira - get-issue with attachment wrong url #17670

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

Merged
merged 16 commits into from
Feb 20, 2022
Merged

Bug Jira - get-issue with attachment wrong url #17670

merged 16 commits into from
Feb 20, 2022

Conversation

MLainer1
Copy link
Contributor

@MLainer1 MLainer1 commented Feb 15, 2022

Contributing to Cortex XSOAR Content

Make sure to register your contribution by filling the contribution registration form

The Pull Request will be reviewed only after the contribution registration form is filled.

Status

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

fixes: https://github.com/demisto/etc/issues/46579

Description

Wrong url when trying to get attachment of an issue when calling command get-issue
image
image

Minimum version of Cortex XSOAR

  • 6.0.0
  • 6.1.0
  • 6.2.0
  • 6.5.0

Does it break backward compatibility?

  • Yes
    • Further details:
  • No

Must have

  • Tests
  • Documentation

@MLainer1 MLainer1 requested a review from a team as a code owner February 15, 2022 09:07
@MLainer1 MLainer1 changed the title Bug Jira - Get issue with attachment wrong url Bug Jira - get-issue with attachment wrong url Feb 15, 2022
@MLainer1 MLainer1 requested review from jochman and removed request for a team February 15, 2022 12:57
@MLainer1 MLainer1 added the release-notes-only Indicates that this pull request has ONLY release notes to review for documentation process label Feb 15, 2022
Copy link
Contributor

@jochman jochman left a comment

Choose a reason for hiding this comment

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

Is that a bug in the integration or a change of the api?

requests_mock.get('https://localhost/rest/api/latest/issue/VIK-267', json=GET_ISSUE_WITH_ATTACHMENT_RESPONSE)
requests_mock.get('https://localhost/rest/api/2/attachment/content/16188', json={})

get_issue("VIK-267", get_attachments="true") # the command will fail if not doing a request with the proper url
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
get_issue("VIK-267", get_attachments="true") # the command will fail if not doing a request with the proper url
assert get_issue("VIK-267", get_attachments="true"), 'There was a request to the wrong url'

Much clearer when fails.

def test_get_issue_attachment_url(mocker, requests_mock):
"""
Given:
- The issue ID.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not describing what you're testing.

@@ -1282,3 +1282,23 @@ def test_get_custom_field_names(mocker, requests_mock):
requests_mock.get('https://localhost/rest/api/latest/field', json=FIELDS_RESPONSE)
res = get_custom_field_names()
assert res == EXPECTED_RESP


def test_get_issue_attachment_url(mocker, requests_mock):
Copy link
Contributor

Choose a reason for hiding this comment

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

Call the function in a describable name.

@@ -673,7 +673,7 @@ def get_issue(issue_id, headers=None, expand_links=False, is_update=False, get_a
if get_attachments and attachments:
attachment_urls = [attachment['content'] for attachment in attachments]
for attachment_url in attachment_urls:
attachment = f"secure{attachment_url.split('/secure')[-1]}"
attachment = f"rest{attachment_url.split('/rest')[-1]}"
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens here?
I suggest you extract it into a new function and test it with input/output so other developers can see what you're doing here.

@MLainer1 MLainer1 requested a review from jochman February 16, 2022 15:35
Copy link
Contributor

@jochman jochman left a comment

Choose a reason for hiding this comment

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

ok

Comment on lines 244 to 258
'aggregatetimeestimate': None, 'summary': 'test master1',
'creator': {
"accountId": "557058:fb80ffc0-b374-4260-99a0-ea0c140a4e76",
"accountType": "atlassian",
"active": True,
"avatarUrls": {
"16x16": "",
"24x24": "",
"32x32": "",
"48x48": ""
},
"displayName": "jon doe",
"emailAddress": "[email protected]",
"self": "https://demistodev.atlassian.net/rest/api/2/user?accountId=id",
"timeZone": "Asia"
}
}
"accountId": "557058:fb80ffc0-b374-4260-99a0-ea0c140a4e76",
"accountType": "atlassian",
"active": True,
"avatarUrls": {
"16x16": "",
"24x24": "",
"32x32": "",
"48x48": ""
},
"displayName": "jon doe",
"emailAddress": "[email protected]",
"self": "https://demistodev.atlassian.net/rest/api/2/user?accountId=id",
"timeZone": "Asia"
Copy link
Contributor

Choose a reason for hiding this comment

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

Be consistent with your quotation marks. They should either all be single quotes or double quotes - not a mixture of the two. The 'soon-to-be' official recommendation (it hasn't yet been added to the development conventions in the development docs yet) is to use single quotes.

"self": "https://demistodev.atlassian.net/rest/api/2/user?accountId=id",
"timeZone": "Asia"
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This bracket's placement is misformatted.


#### Integrations
##### Atlassian Jira v2
- Fixed an issue where calling the command **get-issue** with *getAttachments="true"* returned an error.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Fixed an issue where calling the command **get-issue** with *getAttachments="true"* returned an error.
- Fixed an issue where calling the **get-issue** command with *getAttachments="true"* caused an error.

Comment on lines 237 to 259
'emailAddress': '[email protected]',
'avatarUrls': {
'48x48': ''},
'displayName': 'Meir Wahnon', 'active': True, 'timeZone': 'Asia/Jerusalem',
'accountType': 'atlassian'},
'created': '2021-04-04T12:49:42.881+0300', 'size': 8225,
'mimeType': 'application/json',
}],
'aggregatetimeestimate': None, 'summary': 'test master1',
'creator': {
'accountId': '557058:fb80ffc0-b374-4260-99a0-ea0c140a4e76',
'accountType': 'atlassian',
'active': True,
'avatarUrls': {
'16x16': '',
'24x24': '',
'32x32': '',
'48x48': ''
},
'displayName': 'jon doe',
'emailAddress': '[email protected]',
'self': 'https://demistodev.atlassian.net/rest/api/2/user?accountId=id',
'timeZone': 'Asia'
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like some of this info is real(ish)... make sure to use fake information.

@MLainer1 MLainer1 removed the release-notes-only Indicates that this pull request has ONLY release notes to review for documentation process label Feb 17, 2022
@MLainer1 MLainer1 requested a review from avidan-H February 17, 2022 12:49
@content-bot
Copy link
Collaborator

@MLainer1 MLainer1 merged commit 40ea26a into master Feb 20, 2022
@MLainer1 MLainer1 deleted the Bug_Jira branch February 20, 2022 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants