Skip to content

Adding teams API #1794

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

Closed
wants to merge 37 commits into from
Closed

Conversation

Maxim-Durand
Copy link

#1761

Important

This PR isn't finished as I'm currently stuck not being able to test my changes.
Indeed the jira server container used to test is too old and doesn't support newest API like the Jira Team API or the Jira Org API
Hence, I'm asking the maintainer of this project what is the strategy to keep this project up-to-date with Jira newest APIs ? Do we have a cloud server we could use to test ?

Started adding the tests for those org routes
… calling service desk do it from the same root api endpoint
Started adding the tests for those org routes
… calling service desk do it from the same root api endpoint
Copy link

github-actions bot commented Jan 7, 2024

Label error. Requires exactly 1 of: bug, enhancement, major, minor, patch, skip-changelog. Found: feature

@adehad
Copy link
Contributor

adehad commented Jan 12, 2024

Hi @Maxim-Durand, we do have a Jira Cloud instance. However, the credentials for this are not shared with pull requests due to a limitation within GitHub so the process would likely involve the maintainers running in separately to confirm functionality.

Copy link
Contributor

@adehad adehad left a comment

Choose a reason for hiding this comment

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

lots of effort into this, great start to your first PR

tests/tests.py Outdated
Comment on lines 627 to 628
if self._should_skip_for_pycontribs_instance():
self._skip_pycontribs_instance()
Copy link
Contributor

Choose a reason for hiding this comment

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

We use a test decorator to enable the tests running on the Jira Cloud instance, this is denoted by @allow_on_cloud, an example is in test_group

list[str]
"""
url = f"/gateway/api/public/teams/v1/org/{org_id}/teams/{team_id}/members"
payload = {"first": 50}
Copy link
Contributor

Choose a reason for hiding this comment

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

this pagination implementation should be moved into a helper method, if is is specific to the Teams API I would make it a static method of the Teams Resource

Then you would call it like:

Teams._fetch_pages(session=session, url=url, ...)

I would aim to match as many of the parameters to the function JIRA._fetch_pages() if possible

Copy link
Author

Choose a reason for hiding this comment

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

In theory it's not specific to Teams API, but isn't exactly like JIRA._fetch_pages() neither as it's using different identifiers for Jira's latest API (why they didn't stick to the same variable names between API idk 🙄 )

For now I simply created it as a helper function but if it it's used outside of Teams it's bound to change.

JiraTestCase.setUp(self)
self.test_team_name = f"testTeamFor_{self.test_manager.project_a}"
self.test_team_type = "OPEN"
self.org_id = os.environ["CI_JIRA_ORG_ID"]
Copy link
Author

@Maxim-Durand Maxim-Durand Jan 19, 2024

Choose a reason for hiding this comment

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

I'm not sure how to get the org_id of the testing organisation 🤔

I guess the only real solution is to implement the organisation API and to create a new one before running the tests.
I'll get to work 😉

Copy link
Author

Choose a reason for hiding this comment

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

I created a new PR (#1803) based on this one but also adding Organisations API so that I'm able to test the Team API.

@Maxim-Durand
Copy link
Author

However, the credentials for this are not shared with pull requests due to a limitation within GitHub so the process would likely involve the maintainers running in separately to confirm functionality.

Although I understand the reasoning this seems like a very slow and inefficient process.
Is there no way to simply share an API token with limited permissions and for a limited time-frame so I can actually test this without spamming maintainers ?

@Maxim-Durand
Copy link
Author

#1794 (comment)

I think I'll close this PR in favor of #1803

@adehad
Copy link
Contributor

adehad commented Jan 22, 2024

However, the credentials for this are not shared with pull requests due to a limitation within GitHub so the process would likely involve the maintainers running in separately to confirm functionality.

Although I understand the reasoning this seems like a very slow and inefficient process. Is there no way to simply share an API token with limited permissions and for a limited time-frame so I can actually test this without spamming maintainers ?

I agree the process is not ideal, but this is a limitation from GitHub. Furthermore, it is impossible to share a limited permission token, as the nature of the API calls of the test suite involve Admin permissions.

We reduce the friction by having the Server tests, that once green give an indication to us maintainers whether we should run the Cloud tests.

Typically I would expect contributors to have tried their API with their own instances (or other public instances, as shown in https://github.com/pycontribs/jira/blob/main/examples/basic_use.py). At very least the "get" based methods.

@Maxim-Durand
Copy link
Author

Typically I would expect contributors to have tried their API with their own instances (or other public instances, as shown in https://github.com/pycontribs/jira/blob/main/examples/basic_use.py). At very least the "get" based methods.

Fair enough, I'll do that.

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.

2 participants