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

Introduced TypeScript type definitions SearchJiraResponse and JiraQueryResults to represent Jira search responses and pagination details. #127

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SaachiNayyer
Copy link
Contributor

@SaachiNayyer SaachiNayyer commented May 13, 2024

Introduced TypeScript type definitions SearchJiraResponse and JiraQueryResults to represent Jira search responses and pagination details. Updated the searchJira function to return search results as a SearchJiraResponse, incorporating the new types. Enhanced error handling in the searchJira function by handling HTTP response errors and logging them appropriately. The JiraQueryResults type outlines the structure of a paginated Jira search response, facilitating better data handling. These changes streamline the Jira Dashboard plugin's codebase, improving error resilience and clarity in handling search operations.

Describe your changes

Please include a summary of the change, a relevant motivation and context.

Issue ticket number and link

  • Fixes #(issue)

Checklist before requesting a review

  • I have performed a self-review of my own code
  • I have verified that the code builds perfectly fine on my local system
  • I have added tests that prove my fix is effective or that my feature works
  • I have commented my code, particularly in hard-to-understand areas
  • I have verified that my code follows the style already available in the repository
  • I have made corresponding changes to the documentation

@SaachiNayyer SaachiNayyer requested a review from a team as a code owner May 13, 2024 15:16
Copy link

changeset-bot bot commented May 13, 2024

🦋 Changeset detected

Latest commit: b646a74

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@axis-backstage/plugin-jira-dashboard-backend Major
@axis-backstage/plugin-jira-dashboard-common Minor
backend Patch
app Patch
@axis-backstage/plugin-jira-dashboard Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Member

@GLundh GLundh left a comment

Choose a reason for hiding this comment

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

LGTM, But @anicke should probably take an extra look.

@SaachiNayyer
Copy link
Contributor Author

Hi GLundh @anicke
any thoughts or progress updates? for this PR
Thanks

@SaachiNayyer SaachiNayyer requested a review from GLundh May 20, 2024 09:10
Copy link
Contributor

@anicke anicke left a comment

Choose a reason for hiding this comment

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

Maybe it would be easier to create a new function that will return the complete jira response.

.changeset/witty-jokes-hammer.md Outdated Show resolved Hide resolved
plugins/jira-dashboard-backend/src/api.ts Outdated Show resolved Hide resolved
plugins/jira-dashboard-backend/src/api.ts Outdated Show resolved Hide resolved
@fridajac fridajac mentioned this pull request Jun 20, 2024
6 tasks
@SaachiNayyer SaachiNayyer changed the title Signed-off-by: enaysaa [email protected] Introduced TypeScript type definitions SearchJiraResponse and JiraQueryResults to represent Jira search responses and pagination details. Jun 27, 2024
@SaachiNayyer
Copy link
Contributor Author

Hi anicke
Thanks for your comments. Have updated the PR with the changes. Request your review based on your availability and let me know if any further comments :)

@SaachiNayyer
Copy link
Contributor Author

Hi anicke GLundh
Just a gentle reminder :) I have resolved the given comments.
Please let me know if you have any feedback/concerns based on your availability.
Thanks

…ons by introducing structured TypeScript type definitions and enhancing the searchJira function to return both search results and HTTP status codes

Signed-off-by: enaysaa [email protected]

Introduced TypeScript type definitions SearchJiraResponse and JiraQueryResults to represent Jira search responses and pagination details.
Updated the searchJira function to return search results as a SearchJiraResponse, incorporating the new types.
Enhanced error handling in the searchJira function by handling HTTP response errors and logging them appropriately.
The JiraQueryResults type outlines the structure of a paginated Jira search response, facilitating better data handling.
These changes streamline the Jira Dashboard plugin's codebase, improving error resilience and clarity in handling search operations.
@SaachiNayyer
Copy link
Contributor Author

Hi anicke GLundh
Just a gentle reminder :) I have resolved the given comments.
Its been a while for any update regarding this PR
Please let me know if you have any feedback/concerns based on your availability.
Thanks

@SaachiNayyer
Copy link
Contributor Author

Hi anicke GLundh @fridajac
Any update on this? As we need this PR merged urgently as we want to align our backstage plugins with opensource versions.
And without this commit being merged we are blocked. Happy to discuss in case this commit is missing any functionality or further improvements.
Also #177 is dependent on this PR to be merged.
Appreciate your feedback based on your availability.

Thanks

'@axis-backstage/plugin-jira-dashboard-common': minor
---

Introduced TypeScript type definitions SearchJiraResponse and JiraQueryResults to represent Jira search responses and pagination details.
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this changes the "searchJira" call in a breaking way please update this with code examples how it has changed and how to update any code that is using it.

* @public
*/
export const searchJira = async (
config: Config,
jqlQuery: string,
options: SearchOptions,
): Promise<Issue[]> => {
): Promise<SearchJiraResponse> => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change the error handling a bit? I would prefer if this would return "JiraQueryResults" directly. What is the benefit of having the caller of this function have to check the response status? Maybe something like this?

  const response = await fetch(`${resolveJiraBaseUrl(config)}search`, {
  ....
  }); 
  if (response.ok) {
     return await response.json();
  }
  throw new Error(`${response.status}`)

@SaachiNayyer
Copy link
Contributor Author

Hi @anicke
Any feedback on my previous response. Please let us know your views based on your availability.
Thanks

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

Successfully merging this pull request may close these issues.

3 participants