Added initial insights

[Fank]

What type of PR is this?

feature

What this PR does / why we need it:

Adds cloud app insight: https://developer.atlassian.com/cloud/insight/rest/

Which issue(s) this PR fixes:

Relates to #495

Special notes for your reviewer:

Missing ToDos:

• [ ] ~~Added documentation~~ (will be added later)
• [x] Added tests

Additional documentation e.g., usage docs, etc.:

[andygrunwald]

Thanks for this Pull Request. Overall it looks quite good. I would like to run through a few points with you

model package

What is the thought behind the own model package? The package contains all structs in the top-level package right now.

Are there name conflicts?

Insight API vs. single services

In the current implementation (for object Get objecttype {id} attributes), you call it like

client.Insight.GetObjectTypeAttributes(...)

What do you think about an API like

client.Insight.Objecttype.GetAttributes(...)

The sublevel (here Objecttype) would be in line with the documentation navigation structure at https://developer.atlassian.com/cloud/insight/rest/api-group-objecttype/#api-objecttype-id-attributes-get

Unit tests

It would be nice to have a few (you listed those as open TODOs already)

Checking of status codes

The code

switch res.StatusCode {
case http.StatusUnauthorized:
	return nil, fmt.Errorf("%s: %w", req.URL.String(), ErrUnauthorized)
case http.StatusNotFound:
	return nil, fmt.Errorf("%s: %w", req.URL.String(), ErrNotFound)
case http.StatusInternalServerError:
	return nil, fmt.Errorf("%s: %w", req.URL.String(), ErrUnknown)
}

It looks like it is nearly all the same. Would we be able to make this more generic and place this in the Do method (where the request is executed)?

[Fank]

model package

What is the thought behind the own model package? The package contains all structs in the top-level package right now.

Are there name conflicts?

No there are not (yet), buts its an app from the marketplace for the cloud service and because of that i would like to separate it because its tracks a completely different version and naming. So it easier to find out where are those structs are coming from and why are they there.

Insight API vs. single services

In the current implementation (for object Get objecttype {id} attributes), you call it like

client.Insight.GetObjectTypeAttributes(...)

What do you think about an API like

client.Insight.Objecttype.GetAttributes(...)

The sublevel (here Objecttype) would be in line with the documentation navigation structure at https://developer.atlassian.com/cloud/insight/rest/api-group-objecttype/#api-objecttype-id-attributes-get

Yes i totally missed that, its changed as written above.

Checking of status codes

The code

switch res.StatusCode {
case http.StatusUnauthorized:
	return nil, fmt.Errorf("%s: %w", req.URL.String(), ErrUnauthorized)
case http.StatusNotFound:
	return nil, fmt.Errorf("%s: %w", req.URL.String(), ErrNotFound)
case http.StatusInternalServerError:
	return nil, fmt.Errorf("%s: %w", req.URL.String(), ErrUnknown)
}

It looks like it is nearly all the same. Would we be able to make this more generic and place this in the Do method (where the request is executed)?

This is a proof of concept to check if this is a solid way to handle errors, so far it looks good. I would like to replace the existing checks for bad status code with this behavior in general, because i see the same status codes in Jira REST and Service Desk too. Is this fine to replace the current status code handling with this attempt? If yes i would like to separate it into a new PR

[Fank]

Unit tests

It would be nice to have a few (you listed those as open TODOs already)

Added

[Fank]

This is the first batch and contains all functions i need at the moment.

[Fank]

Checking of status codes

The code

switch res.StatusCode {
case http.StatusUnauthorized:
	return nil, fmt.Errorf("%s: %w", req.URL.String(), ErrUnauthorized)
case http.StatusNotFound:
	return nil, fmt.Errorf("%s: %w", req.URL.String(), ErrNotFound)
case http.StatusInternalServerError:
	return nil, fmt.Errorf("%s: %w", req.URL.String(), ErrUnknown)
}

It looks like it is nearly all the same. Would we be able to make this more generic and place this in the Do method (where the request is executed)?

Will be changed with #570 , i would keep the current handling and replace it in the linked PR.

[andygrunwald]

Thanks. This PR is still on my list to review and move forward. It might take a few days.

[ChristianKniep]

Hey there, what is the status of the asset integration. I am going to need that soon(-ish), so happy to help. EDIT: Commenting before reading the complete information... there are other PRs already which are newer. :)

[Fank]

@ChristianKniep this PR is abandoned Currently working on that here https://github.com/ctreminiom/go-atlassian/pull/204

[andygrunwald]

Just Reopening this. I think there are a few good ideas in this PR. Right now, my time is pretty limited, still, I plan to come back to this.