go-jira icon indicating copy to clipboard operation
go-jira copied to clipboard

#368 Added warningMessages to issue.search() response

Open manuelbcd opened this issue 3 years ago • 5 comments

Description

When using Issue.Search(...) response can contain optional field "WarningMessages" that was not implemented in the response struct.

This feature allows to manage possible WarningMessages since jql errors are not rendered as errors but as warning messages. Without this option, you are unable to find out if your jql query failed because i.e. specified user does not exists. More information in #368

Information that is useful here:

  • The What: Adding WarningMessages[] to searchResult (search response wrapper)
  • The Why: It is described at #368 (i.e. user specified in jql query does not exist)
  • Type of change: New feature.
  • Breaking change: Yes. My implementation adds a new field to issue.Search() return. If you think there is a better implementation approach, please suggest.
  • Related to an issue: #368
  • Jira Version + Type: Cloud

Example:

Let us know how users can use or test this functionality.

chunk, resp, warningMessages, err := client.Issue.Search(searchString, opt)
if err != nil {
	return nil, err
}
if len(warningMessages) > 0 {
  fmt.Printf("Warning messages in response:\n")
  for i, warn := range warningMessages {
	fmt.Printf("Warning %d: %s\n", i, warn)
  }
}

Checklist

manuelbcd avatar Mar 23 '21 23:03 manuelbcd

LGTM, but we may want to have some notification around merging it. It looks like it changes the behavior around search as defined in the code that uses the library and could possibly break peoples code unless they handle for the new output variable. thoughts @andygrunwald

kusuriya avatar Mar 24 '21 04:03 kusuriya

Yes @kusuriya, i do agree. Im worried because im not sure if is worth adding this feature while breaking code compatibility. Actually I was asking about that in #368

If someone can give it a thought... maybe there could be a different approach

manuelbcd avatar Mar 24 '21 07:03 manuelbcd

Thanks for the effort, the PR, and for the enhancement. Indeed, the current implementation is breaking the compatibility. We have a few PRs open that will break the existing interfaces, and this is the main reason why those doesn't have been merged yet. For quite some time, we are talking about an official v2, that fixes many of these implementation details. I see two options here:

  1. Keeping this PR for v2 (no timeline yet)
  2. Changing the implementation into a compatible way

About 2), one idea would be to create a new Search function that supports these warning messages. When a new Search functionality is created, I would even suggest changing the type that is returned to something like

type SearchResult struct {
    Issues []issue ...
    WarningMessage []messages ...
    ....
}

By this, we would not break the compatibility in the future, if a new field is part of the structure. I am curious about what you think and your feedback.

andygrunwald avatar Mar 25 '21 13:03 andygrunwald

Thanks for asking. In my opinion, adding a new function just to bring the ability to access to hypothetical warning messages could be a bit complex for library consumers... I bet for the 1st one, implement it into v2 refining implementation details first.
However if we receive more issues and votes to add the feature to v1, we can always add this third function in the future (in 3 weeks, in 3 months, it does not matter).

manuelbcd avatar Mar 25 '21 15:03 manuelbcd

Hey,

I am very sorry that this pull request has been open for a long time with no final feedback or merge/decline. We work on this project in our spare time, and sometimes, other priorities take over. This is the typical open source dilemma.

However, there is news: We are kicking off v2 of this library 🚀

To provide visibility, we created the Road to v2 Milestone and calling for your feedback in https://github.com/andygrunwald/go-jira/issues/489

The development will take some time; however, I hope you can benefit from the changes. If you seek priority development for your pull request + you like to sponsor it, please contact me.

What does this mean for my pull request?

We will work on this pull request indirectly. We might merge it during the development or pull parts of it into the new version. This means that during the development phase, we aim to tackle it. Maybe in a different way like it is currently handled. Please understand that this will take a while because we are running this in our spare time.

Final words

Thanks for using and contributing to this library. If there is anything else you would like to tell us, let us know!

andygrunwald avatar Aug 21 '22 15:08 andygrunwald