Skip to content

Change setup validation to use a dto - #1852

Merged
Hinton merged 1 commit into
masterfrom
hotfix/setup-master
Feb 9, 2022
Merged

Change setup validation to use a dto#1852
Hinton merged 1 commit into
masterfrom
hotfix/setup-master

Conversation

@Hinton

@Hinton Hinton commented Feb 9, 2022

Copy link
Copy Markdown
Member

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

Changes the setup application to use a DTO for the response from the bitwarden server. This ensures we use the System.Text.Json Web defaults https://docs.microsoft.com/en-us/dotnet/api/system.text.json.jsonserializerdefaults?view=net-6.0#system-text-json-jsonserializerdefaults-web which are case-insensitive. It's also nicer than using JsonDocument.

Testing requirements

Verify setup works against prod. (Would be nice to verify it correctly checks Disabled)

Before you submit

  • I have checked for formatting errors (dotnet tool run dotnet-format --check) (required)
  • If making database changes - I have also updated Entity Framework queries and/or migrations
  • I have added unit tests where it makes sense to do so (encouraged but not required)
  • This change requires a documentation update (notify the documentation team)
  • This change has particular deployment requirements (notify the DevOps team)

@Hinton
Hinton requested review from a team and justindbaur February 9, 2022 15:52

@justindbaur justindbaur left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Love it!

@Hinton
Hinton merged commit 6f2f475 into master Feb 9, 2022
@Hinton
Hinton deleted the hotfix/setup-master branch February 9, 2022 16:13
Hinton added a commit that referenced this pull request Feb 9, 2022
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