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

refactor: swaps implementation from deprecated methods to new methods #112

Merged
merged 8 commits into from
Dec 3, 2024

Conversation

ctran88
Copy link
Contributor

@ctran88 ctran88 commented Nov 26, 2024

What's New?

  • the deprecated methods now wrap the new methods
  • moves validate jwt and create magic link methods to Auth struct
  • aliases some types to new names

Screenshots (if appropriate):

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have manually tested my code thoroughly
  • I have added/updated inline documentation for public facing interfaces if relevant
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing integration and unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Additional context

@ctran88 ctran88 requested a review from a team November 26, 2024 23:26
return nil, Error{Message: "network error: failed to create Passage Magic Link"}
var passageError PassageError
if errors.As(err, &passageError) {
return magicLink, Error{
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 mapping (same for all the deprecated methods) is technically a breaking change since it drops the StatusText value. but since that value is basically just the generic reason for a status code (unlikely that anyone would be checking this), i figured it would be okay to drop.

app_user.go Outdated
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 contents of this file is just the original contents of user.go. it was renamed in 79dae32

it was renamed so this whole file can be deleted in the major release.

and technically the name fits better for the user operations that are receiver functions of App

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 file will be renamed to passage.go in the major release when App is replaced

app_user_test.go Outdated
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 is just a file rename from 79dae32

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 file will be deleted in the major release

user.go Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

rename of app_user.go from 79dae32

user_test.go Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just a file rename from 79dae32

@ctran88 ctran88 force-pushed the PSG-5425-update-new-methods branch from 0fb435d to 1bd7679 Compare November 27, 2024 00:04
return strings.TrimSuffix(sb.String(), ", ")
}

func errorFromResponse(body []byte, statusCode int) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since each generated client call returns its own struct (i.e. GetUserResponse, ActivateUserResponse, etc) and the required values are only exposed as properties (JSON400.Error or JSON400.Code)

it can't be easily abstracted into a generic function without delineating each of the different possible types and casting them.

so instead this function manually unmarshalls the response body instead of using the pre-parsed JSONXXX properties to DRY up the code.

the trade-off is that it's taking extra steps to parse the bytes into a new struct, but i thought the simplification was worth it.

@ctran88 ctran88 mentioned this pull request Dec 2, 2024
12 tasks
Copy link

sonarqubecloud bot commented Dec 3, 2024

@ctran88 ctran88 changed the title feat: swaps implementation from deprecated methods to new methods refactor: swaps implementation from deprecated methods to new methods Dec 3, 2024
@ctran88 ctran88 merged commit 5b1772c into main Dec 3, 2024
6 checks passed
@ctran88 ctran88 deleted the PSG-5425-update-new-methods branch December 3, 2024 17:29
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.

2 participants