Skip to content

Application custom config #71

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

Merged
merged 8 commits into from
Mar 28, 2024

Conversation

jworkmanjc
Copy link
Contributor

@jworkmanjc jworkmanjc commented Mar 18, 2024

Issues

  • CUT-3922 - Add additional application (sso) properties to

What does this solve?

The default models defined in our API specification are missing several properties in the Application model, This release manually re-defines the config model for the Application model.

Is there anything particularly tricky?

I've added a new "find and replace" type of way to redefine the generated code. Instead of using regex, to find and replace many things, you just need to replace sections of the API model. Within the apiTransform.ps1 file I've added a OverrideDefinitions list. Within that list, one can denote a part of the SDK swagger spec to replace entirely. All you would need to do that would be to define a .json file with the same name in the /Custom directory.

How should this be tested?

Using the current version of the V2 SDK (0.0.43) running the command: remove-JCSystemGroupMember -SystemID 62ace7fbac30e538a6ba0ec8 -Groupid 604938c5232e1179d1e7bbdb should return multiple status messages
Screenshot 2024-03-28 at 9 31 12 AM

Using this version of the V2 SDK (0.0.44) running the command: remove-JCSystemGroupMember -SystemID 62ace7fbac30e538a6ba0ec8 -Groupid 604938c5232e1179d1e7bbdb should return the single status message as expected
Screenshot 2024-03-28 at 9 30 45 AM

503 error should still catch and recover 😎
Screenshot 2024-03-28 at 9 32 43 AM

Screenshots

@jworkmanjc jworkmanjc added patch Patch SDK Release Version SDK SDK Release Label labels Mar 18, 2024
@jworkmanjc jworkmanjc marked this pull request as ready for review March 20, 2024 21:09
@jworkmanjc jworkmanjc requested a review from a team as a code owner March 20, 2024 21:09
@@ -611,7 +617,7 @@ Function Update-SwaggerObject {
} Else {
$excludePath = ($InputObjectName -replace '.paths.', '') -replace "\.\w+", ''
# Write-Warning "[status] New SDK Endpoint Found: `nOperationId: $($ThisObject.operationId) `nPath: $excludePath"
Write-Error ("In '$($CurrentSDKName)' unknown operationId '$($ThisObject.operationId) - $($InputObjectName)'.")
Write-Warning ("In '$($CurrentSDKName)' unknown operationId '$($ThisObject.operationId) - $($InputObjectName)'.")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This really didn't need to throw an error/ switched to write-warning

@@ -69,6 +69,9 @@ $TransformConfig = [Ordered]@{
'\[,' = '[';
',]' = ']';
};
OverrideDefinitions = @(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the idea here is that if you want to replace an entire section of the Swagger spec you could do that by specifying the path of item you wish to replace and adding a corresponding json file in /CustomDefinitions/ directory

gweinjc
gweinjc previously approved these changes Mar 21, 2024
Copy link
Contributor

@gweinjc gweinjc left a comment

Choose a reason for hiding this comment

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

Looks good to me, nice job with the added method of replacing things in the future if needed. I'm sure that'll come in handy!

kmaranionjc
kmaranionjc previously approved these changes Mar 26, 2024
@jworkmanjc jworkmanjc dismissed stale reviews from kmaranionjc and gweinjc via ae85055 March 27, 2024 17:56
@gweinjc gweinjc self-requested a review March 27, 2024 19:34
gweinjc
gweinjc previously approved these changes Mar 27, 2024
@gweinjc gweinjc self-requested a review March 27, 2024 19:35
Copy link
Contributor

@gweinjc gweinjc left a comment

Choose a reason for hiding this comment

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

Looks good, nice job with the custom definitions. It'll make future work much easier

Copy link
Contributor

@kmaranionjc kmaranionjc left a comment

Choose a reason for hiding this comment

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

Nice work! Looks good on my end

@jworkmanjc jworkmanjc merged commit a299cf9 into master Mar 28, 2024
@jworkmanjc jworkmanjc deleted the CUT-3922_Application_missing_properties branch March 28, 2024 17:36
@jworkmanjc jworkmanjc temporarily deployed to PublishToPSGallery March 28, 2024 17:38 — with GitHub Actions Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Patch SDK Release Version SDK SDK Release Label
Development

Successfully merging this pull request may close these issues.

3 participants