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

Allow custom field values to be specified as JSON #661

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

Conversation

octachrome
Copy link

This PR adds the ability to update custom fields whose type is not directly supported by jira-cli. This is achieved by specifying the JSON representation of the field value, e.g.:

jira issue create --custom 'json:found-in-version={"name":"Product 1.4"}'

jira issue edit ISSUE-123 --custom 'json:team={"set":"My Team"}' \
    --custom 'json:assigned-developer={"set":{"name":"MyUsername"}}'

If the field value starts with json:, the remaining string is interpreted as raw JSON. Note that when updating an issue, the set or add operation must also be provided, since this can be different for each custom field.

The implementation uses a new custom field struct type, customFieldTypeJson, and an implementation of MarshallJSON for that type which just returns the JSON stored in the struct.

After I implemented this I found #593 suggesting the same idea.

Copy link
Owner

@ankitpokhrel ankitpokhrel left a comment

Choose a reason for hiding this comment

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

Hi @octachrome, Thank you for working on this.

I think we can get rid of json: prefix (unless I am missing something). Let me know what you think! Thanks again!

if strings.HasPrefix(key, "json:") {
key = key[5:]
rawJson = true
}
for _, configured := range configuredFields {
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like we still need to go through configured field so why not just add a type called json that accepts the raw json data?

For instance the configuration can be:

issue:
    fields:
        custom:
            - name: platform
              key: customfield_10032
              schema:
                datatype: json

And we can pass it as is

Screen Shot 2023-09-13 at 9 18 39 PM

Now, we don't need to pass json: tag?

jira issue create --custom='platform=[{"value":"Linux"}]'

Copy link
Author

Choose a reason for hiding this comment

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

It is a good idea to have a json type that can be used in the config file. But is there an easy way to update the config file to set the type of custom fields? I think the Jira users on my team will find it hard to locate this file and make the right change by hand. Another inconvenience is that if a user runs jira init (for example if their password/access token changes), then the config file will be reset.

Looking at my config file, all the custom fields for which JSON is needed have type any. What do you think about using JSON by default for all fields of type any?

Another idea is to add an option to the jira init command to mark certain fields as needing JSON, e.g.:

jira init --json-custom-fields "first-field,second-field,etc"

Copy link
Owner

Choose a reason for hiding this comment

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

But is there an easy way to update the config file to set the type of custom fields?

Not at the moment unfortunately. There is a plan to have something like gh config but I don't have any timeline for it yet (open for PRs :)).

I think the Jira users on my team will find it hard to locate this file and make the right change by hand.

Unless I am missing something, even with the json: tag you will still need to configure the field in config right? since we are looping through the configured fields in line#236?

for _, configured := range configuredFields {

Looking at my config file, all the custom fields for which JSON is needed have type any. What do you think about using JSON by default for all fields of type any?

Another idea is to add an option to the jira init command to mark certain fields as needing JSON, e.g.:

These are all valid ideas. The inconsistencies in Jira could make things a bit tricky so I am not sure if this will be the same case for all Jira installations.

I would suggest, let's start with simple and working implementation and we can make it better iteratively.

Copy link
Author

Choose a reason for hiding this comment

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

you will still need to configure the field in config right

Running jira init populates the config file with all custom fields, even the ones with unknown (any) type, so no manual config is necessary.

The current default behaviour for any type fields is to just send a string. I would rather not change that default behaviour in case it breaks something for an existing user (unless you know that this never happens in practice).

So I propose to add json as a new custom field type and add a flag to jira init to make it easy to configure these fields.

Copy link
Owner

Choose a reason for hiding this comment

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

Sounds good! Let's start by adding json as a new custom field type. We can talk about how we can make the config easily configurable.

Copy link
Author

Choose a reason for hiding this comment

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

Here is my second attempt.

  • Added json as a custom type, which can be used in the config file to force a field to use JSON when setting/editing.
  • JSON is used by default for unknown field types, such as user or version.
  • Factored out the code which constructs the customFieldTypeXxx objects so that it can be called recursively for arrays of any type, including JSON.
  • Replaced the type-specific structs used to edit custom fields on issues, such as customFieldTypeNumberSet and customFieldTypeStringSet with generic structs that can be used to set/add/remove items on any field type.

I tested this on our Jira installation with custom fields of type option, user, version, date, number, string, any and array of string. This worked nicely, and I could specify user and version types as JSON from the command line with no manual changes to the config file needed.

Copy link
Author

Choose a reason for hiding this comment

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

@ankitpokhrel Have you had a chance to review my update?

Copy link
Owner

Choose a reason for hiding this comment

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

Hi @octachrome, sorry for the late response. Would you be able to provide some examples on how to construct valid requests for different types of custom fields? Thanks!

Copy link
Author

Choose a reason for hiding this comment

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

@ankitpokhrel, if you mean CLI examples, see below (the syntax is the same for the issue create command):

# Field of type "version"
jira issue edit --no-input --custom 'found-in-version={"name": "MyProduct 1.0"}' XYZ-1234
# Field of type "user"
jira issue edit --no-input --custom 'assigned-developer={"name": "BobFossil"}' XYZ-1234

I could add these to the readme, or it might be better to add them to the article here: #346.

@JacobCallahan
Copy link

Would it be possible for the entire custom argument to be json?
--custom '{"my field":"some value","another field":17}'

@octachrome
Copy link
Author

Would it be possible for the entire custom argument to be json? --custom '{"my field":"some value","another field":17}'

The disadvantage is that we would need to parse the JSON, separate out the fields, then reassemble the individual JSON pieces to send to Jira, which is awkward. What would be the benefit of this approach?

@JacobCallahan
Copy link

There are two main advantages in my opinion.

  1. The current proposal makes sense when considering that the value is json and not the field. However, the string syntax passed into custom is an awkward mix between non-json and json. It reads more like toml.
    proposed:
    --custom='platform=[{"value":"Linux"}]'

  2. In order to use this, you have to edit pre-defined fields and set the type to json. This doesn't allow free-form data passed as truly custom fields in the request.

With this said, I fully acknowledge that the proposed change may not be possible with the proposed PR, hence the question.

Thanks for entertaining it!

@octachrome
Copy link
Author

octachrome commented Jan 15, 2024

when considering that the value is json and not the field

I don't understand how anything except the value can be JSON. How can the field be JSON? Can you give an example?

you have to edit pre-defined fields and set the type to json

I agree this is the main disadvantage, and your suggestion does overcome that with a nicer syntax than what I originally proposed.

This doesn't allow free-form data passed as truly custom fields in the request.

Please give examples of what you mean by "free-form data" and "truly custom fields".

I am using the Jira CLI to automate actions in shell scripts, and I expect other people are too. Assembling a single JSON object to describe all my custom field values is not easy with a shell script, keeping track of where commas are needed, etc. An extension of your suggestion is to allow the option to be passed more than once. So both of the following would be equivalent.

--custom-json '{"field1": "value1", "field2": "value2"}'
--custom-json '{"field1": "value1"} --custom-json '{"field2": "value2"}'

I would also use a different option name to avoid confusion with --custom.

@abhisheksoni27
Copy link

any update on this one? this will make things much easier when transitioning issues, updating custom fields etc. @ankitpokhrel @octachrome

@josete89
Copy link

josete89 commented Aug 6, 2024

Is there any plan to merge this at any point?

@ankitpokhrel
Copy link
Owner

Sorry I got distracted with other priorities for a while now. I'll check if this can be merged soon.

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.

5 participants