Skip to content

[PS 920] Fix selfhosted url validations#1967

Merged
LRNcardozoWDF merged 10 commits intomasterfrom
bug/PS-920-fix-selfhosted-url-validations
Jul 11, 2022
Merged

[PS 920] Fix selfhosted url validations#1967
LRNcardozoWDF merged 10 commits intomasterfrom
bug/PS-920-fix-selfhosted-url-validations

Conversation

@LRNcardozoWDF
Copy link
Member

Type of change

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

Objective

Added feedback to user when saving a bad formed URL in Settings page or trying to perform login with a previously saved bad formed url.

Code changes

  • src/App/Pages/Accounts/EnvironmentPage.xaml: Refactor to use AsyncCommand;
  • src/App/Pages/Accounts/EnvironmentPage.xaml.cs: Since AsyncCommand has proper validation to ensure multiple clicks aren't allowed I removed this validation;
  • src/App/Pages/Accounts/EnvironmentPageViewModel.cs: Added validation for URLs saved and show pop up with error message;
  • src/App/Resources/AppResources.resx: Added string with error message for when user tries to save a bad formed URL;
  • src/Core/Services/ApiService.cs: Added validation for bad formed URL previously saved in Settings and show pop up with error. (Note: Since Bit.Core doesn't have access to src/App/Resources/AppResources.resx the message is hard coded for now).

Screenshots

Before you submit

  • I have checked for formatting errors (dotnet tool run dotnet-format --check) (required)
  • 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)

@LRNcardozoWDF LRNcardozoWDF requested a review from fedemkr June 29, 2022 11:02
@LRNcardozoWDF LRNcardozoWDF self-assigned this Jun 29, 2022
@LRNcardozoWDF LRNcardozoWDF marked this pull request as ready for review June 29, 2022 11:05
Copy link
Member

@fedemkr fedemkr left a comment

Choose a reason for hiding this comment

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

A couple of things to improve and on the PR:

  • Change the title to [PS-920] Fix environment url validations so the format is alike the other PRs.
  • On the description, I'd clarify that Settings page is the environment's one or it could be misleading to the Settings tab in the app's home.

@LRNcardozoWDF LRNcardozoWDF changed the title Bug/ps 920 fix selfhosted url validations [PS 920] Fix selfhosted url validations Jul 4, 2022
@LRNcardozoWDF LRNcardozoWDF requested a review from fedemkr July 5, 2022 10:40
Copy link
Member

@fedemkr fedemkr left a comment

Choose a reason for hiding this comment

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

Good work, just one thing I forgot to mention on the first review, and even though the method didn't have exception handling before, we should add it.

* Added generic error message string to AppResources
@LRNcardozoWDF LRNcardozoWDF requested a review from fedemkr July 8, 2022 14:04
fedemkr
fedemkr previously approved these changes Jul 8, 2022
# Conflicts:
#	src/App/Resources/AppResources.Designer.cs
#	src/App/Resources/AppResources.resx
@LRNcardozoWDF LRNcardozoWDF merged commit d621a5d into master Jul 11, 2022
@LRNcardozoWDF LRNcardozoWDF deleted the bug/PS-920-fix-selfhosted-url-validations branch July 11, 2022 17:02
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.

Android app version 2.15.0 - crashes when white space exists in custom (on-prem) server URL.

2 participants