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

fix(Case Management): re-gen service and add examples #79

Merged
merged 15 commits into from
Feb 18, 2021

Conversation

pyrooka
Copy link
Member

@pyrooka pyrooka commented Feb 10, 2021

PR summary

This PR adds examples to the Case Management service.

PR Checklist

Please make sure that your PR fulfills the following requirements:

  • The commit message follows the Angular Commit Message Guidelines.
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • New tests
  • Build/CI related changes
  • Documentation content changes
  • Other (please describe)

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@padamstx padamstx self-requested a review February 10, 2021 22:18
@padamstx
Copy link
Member

One initial comment... when this PR is squashed and merged, let's use a "fix(Case Management): ..." type commit message so that we end up with a new patch version of the sdk (i.e. 0.17.7 -> 0.17.8).

createCaseOptions.SetSeverity(1)
createCaseOptions.SetEu(&casemanagementv1.CasePayloadEu{Supported: core.BoolPtr(true), DataCenter: core.Int64Ptr(123)})

offeringType, _ := caseManagementService.NewOfferingType(casemanagementv1.OfferingTypeGroupCRNServiceNameConst, "cloud-object-storage")
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be good to move the construction of the dependent models (offeringType and offeringPayload) to immediately before the construction of the options model. So the general pattern would be to construct dependent models first, then construct the options model and set various fields on it.

@pyrooka pyrooka changed the title feat(Case Management): add examples fix(Case Management): re-gen service and add examples Feb 12, 2021
Copy link
Member

@padamstx padamstx left a comment

Choose a reason for hiding this comment

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

Left a few comments which we can discuss

BeforeEach(func() {
shouldSkipTest()
})
It(`GetCases request example`, func() {
Copy link
Member

Choose a reason for hiding this comment

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

Let's move this It() block below the one for the update operation... that way the general pattern in terms of order of the examples would be create, get, update, list, delete (there's no delete for this service however).

"This is an example case description.",
)
createCaseOptions.SetSeverity(1)
createCaseOptions.SetEu(&casemanagementv1.CasePayloadEu{Supported: core.BoolPtr(true), DataCenter: core.Int64Ptr(123)})
Copy link
Member

Choose a reason for hiding this comment

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

Let's be a little more explicit with the model creation by first constructing an instance of CasePayLoadEu, then calling SetEu() to set it (similar to how the offeringType and offeringPayload objects are constructed above).

createCaseOptions := caseManagementService.NewCreateCaseOptions(
"technical",
"Example technical case",
"This is an example case description.",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"This is an example case description.",
"This is an example case description. This is where the problem would be described.",


statusPayloadModel := &casemanagementv1.ResolvePayload{
Action: core.StringPtr("resolve"),
Comment: core.StringPtr("It was actually a mistake"),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Comment: core.StringPtr("It was actually a mistake"),
Comment: core.StringPtr("The problem has been resolved."),


addCommentOptions := caseManagementService.NewAddCommentOptions(
"CS1234567",
"This is a test comment",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"This is a test comment",
"This is an example comment",

}

updateCaseStatusOptions := caseManagementService.NewUpdateCaseStatusOptions(
"CS1234567",
Copy link
Member

Choose a reason for hiding this comment

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

Use a variable for this (obtained from the create operation).

panic(err)
}
if result != nil {
defer result.Close()
Copy link
Member

Choose a reason for hiding this comment

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

print contents of file to console (see GC PR for example)


})
It(`RemoveWatchlist request example`, func() {
// begin-removeWatchlist
Copy link
Member

Choose a reason for hiding this comment

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

Move this block under add watchlist example

addResourceOptions := caseManagementService.NewAddResourceOptions(
"CS1234567",
)
addResourceOptions.SetNote("This is a test note")
Copy link
Member

@padamstx padamstx Feb 12, 2021

Choose a reason for hiding this comment

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

set CRN field to crn:mycloud:myservice:123

addResourceOptions := caseManagementService.NewAddResourceOptions(
"CS1234567",
)
addResourceOptions.SetNote("This is a test note")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
addResourceOptions.SetNote("This is a test note")
addResourceOptions.SetNote("This resource is the service that is having the problem.")

Copy link
Member

@padamstx padamstx left a comment

Choose a reason for hiding this comment

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

LGTM

@padamstx padamstx merged commit 0ce3946 into main Feb 18, 2021
@padamstx padamstx deleted the feat-add-examples branch February 18, 2021 20:55
ibm-devx-automation pushed a commit that referenced this pull request Feb 18, 2021
## [0.17.9](v0.17.8...v0.17.9) (2021-02-18)

### Bug Fixes

* **Case Management:** re-gen service and add examples ([#79](#79)) ([0ce3946](0ce3946))
@ibm-devx-automation
Copy link

🎉 This PR is included in version 0.17.9 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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.

3 participants