Skip to content
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

PORT-5598 | Jira integration - support sprints | Change priority to name #804

Open
wants to merge 85 commits into
base: main
Choose a base branch
from

Conversation

lordsarcastic
Copy link
Contributor

@lordsarcastic lordsarcastic commented Jul 10, 2024

Description

  • Added support for sprints
  • Change Issue kind priority field to use the priority name instead of the priority id
  • Added "total issues" field to project kind

Type of change

  • New feature (non-breaking change which adds functionality)
    image

More information can be found in the docs PR.

@lordsarcastic lordsarcastic changed the title PORT-5598 | Jira integration - support sprints | Change priority to webhook PORT-5598 | Jira integration - support sprints | Change priority to name Jul 10, 2024
@lordsarcastic lordsarcastic marked this pull request as ready for review July 16, 2024 10:17
integrations/jira/client.py Outdated Show resolved Hide resolved
integrations/jira/client.py Outdated Show resolved Hide resolved
integrations/jira/client.py Outdated Show resolved Hide resolved
integrations/jira/client.py Outdated Show resolved Hide resolved
integrations/jira/main.py Outdated Show resolved Hide resolved
integrations/jira/client.py Outdated Show resolved Hide resolved
integrations/jira/integration.py Outdated Show resolved Hide resolved
integrations/jira/integration.py Outdated Show resolved Hide resolved
@mk-armah
Copy link
Member

mk-armah commented Aug 9, 2024

Can you add screenshots to the description ?

@matanl490 matanl490 self-requested a review August 13, 2024 11:29
Copy link
Contributor

@PeyGis PeyGis left a comment

Choose a reason for hiding this comment

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

left some comments

integrations/jira/.port/resources/blueprints.json Outdated Show resolved Hide resolved
integrations/jira/CHANGELOG.md Outdated Show resolved Hide resolved
integrations/jira/client.py Outdated Show resolved Hide resolved
Copy link
Contributor

@PeyGis PeyGis left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@erikzaadi erikzaadi left a comment

Choose a reason for hiding this comment

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

Looks great, is there a way to add tests to verify this change?

@lordsarcastic
Copy link
Contributor Author

Looks great, is there a way to add tests to verify this change?

Nothing's impossible. I'll write some tests for it then

@github-actions github-actions bot added size/XL and removed size/L labels Oct 18, 2024
INTEGRATION_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), "../"))


async def test_full_sync_using_mocked_3rd_party(
Copy link
Contributor

Choose a reason for hiding this comment

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

rename method for what exactly you are testing

"calculationProperties": {}
"mirrorProperties": {},
"calculationProperties": {},
"relations": {}
},
{
"identifier": "jiraIssue",
Copy link
Contributor

Choose a reason for hiding this comment

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

I am thinking maybe we should add a property with sprint, where the description of the property will point the user to the docs?

Comment on lines +15 to +20
- Added a field to display total issues in a project (0.1.96)
- Added support for ingesting other fields apart from the default fields (Jira Sprint support) (0.1.96)

### Improvements

- Changed issue priority from id to name (0.1.96)
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
- Added a field to display total issues in a project (0.1.96)
- Added support for ingesting other fields apart from the default fields (Jira Sprint support) (0.1.96)
### Improvements
- Changed issue priority from id to name (0.1.96)
- Added a field to display total issues in a project
- Added support for ingesting other fields apart from the default fields (Jira Sprint support)
### Improvements
- Changed issue priority from id to name

@@ -20,7 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Improvements

- Bumped ocean version to ^0.12.6
- Bumped ocean version to ^0.12.6 (0.1.94)
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
- Bumped ocean version to ^0.12.6 (0.1.94)
- Bumped ocean version to ^0.12.6

Copy link
Contributor

Choose a reason for hiding this comment

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

test should include actual test of expected data and enrichments.
e.g.
testing that when passing a jql then its adds the jql to the query. When adding fields that those fields are being added to the entity and that you see the expected value.

Comment on lines +96 to +100
if delete_action:
issue = data["issue"]
else:
issue = await client.get_single_issue(data["issue"]["key"])
await ocean_action(ObjectKind.ISSUE, [issue])
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should perform a request to get the issue with the fields as well, so we don't override it with null once we receive an event.
Maybe JQL as well? as we might receive an event of an issue that we are not interested which in regular resync wouldn't been inserted, but in real time event it would've.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants