Skip to content

Settings system refactoring#132

Merged
TheSylence merged 115 commits into
masterfrom
SettingsSystemRefactoring
Jun 10, 2020
Merged

Settings system refactoring#132
TheSylence merged 115 commits into
masterfrom
SettingsSystemRefactoring

Conversation

@TheSylence
Copy link
Copy Markdown
Member

@TheSylence TheSylence commented Feb 25, 2020

Current progress for #56

  • The settings menu has been replaced with a "classic" viewmodel without reflect settings magic.
  • The connections and projects viewmodel has been replaced by a "classic" view model and renamed to "Setup"
  • A connection no longer consists of two plugin configuration instances but one and now includes a ConnectionType enum to distinguish Source and Build Plugin connections.
  • An API for defining the configuration for a plugin is more or less done and is used in the Azure DevOps plugin.
    • These options are wrapped in ViewModels and displayed in the setup view
  • Asynchronous execution of the Test Connection command
  • Loading and saving connections is more or less done.
  • Project configuration is not done.
  • Branch name extraction in plugin #129 might be affected by this changes
  • CommandOption has to be implemented and tested
  • UnitTests for the Option API in PluginInterfaces
  • UnitTests for the Configuration ViewModels
  • UnitTests for (Plugin)OptionViewModels
  • PluginOptionViewModels with OptionViewModels are more or less the same. The POVMs could probably inherit OVM. This would allow to reduce the number of DataTemplates (which are more or less duplicates anyway)
  • Extend DummyBuildServer to handle two pipes at once
  • Asynchronous loading of available values for ListOptions
    • Offer functionality to the plugin to implement a "cancel and restart task with new values" functionality to react on user input.
  • Make sure that an OAuth Flow can be implemented with the Option API
  • Add missing Option types that may be needed for other plugins
    • Collection Option
    • Guid Option The user should never enter a Guid by hand. For selecting an object (that could be a simple guid + name) ListOption already does this.
  • Changing a connection no longer implicitly persists to configuration. Instead changes needs to be saved using the new save button.
  • Cleanup. Currently everything is more a less a mess.
    • Note: The TODO comments "why is this here" is just a reminder for me to look at why this is there because it confused me at first sight
    • Get rid of all ReSharper warnings
    • Get rid of new Codacy warnings on this branch
  • Investigate if there is a way to have generic types in ListOption (and the corresponding ViewModels) and do NOT have to suppress Xaml.BindingWithContextNotResolved in OptionResources
  • Review of the new XAML resources to keep a consistent approach
  • Testing

Adding this here because these comments would be lost behind the "load more" button in this conversation...

  • Bug: The list of collections you can select on a project is not updated when the available connections are modified.
  1. Have 1 connection C1 and 1 project P configured at startup
  2. Delete C1

C1 is still in the list of available connections when modifying P
The configured connections are also not removed from the project, when the connection is deleted

  • Bug: Null reference exception when selecting any connection within a project:
    image

  • Bug: When deleting a project the confirmation popup does not show the name of the project but instead the full class name of TextOptionViewModel

  • Bug: Connections are not persisted unless an option is changed

  1. Create new connection
  2. Select type
  3. Click "Close & resume"
  4. Restart BN

The connection is not there anymore

  • Bug: Names of enum values within slide-over settings are not localized

  • Bug: Properties are set to multiple instances of connections

  • Use predefined.json with 4 DummyPlugin connections:
    predefined.zip

  • Go into settings, quickly click through connections

  • Edit Port of "Dummybuild2" to 9999

The port of "DummyBuild" was also changed
It seems the the two instances are the same. Changing the port on one of them, changes the other

  • Bug: The automatically created project does not make sense
  1. Use predefined.json with 4 DummyPlugin connections:
    predefined.zip
  2. Delete config.json
  3. Start BN and go into settings

The automatically created project is called "DummySource" and has only a source control set
image

  • Bug: Automatically created project for predefined connections is created twice
  1. Use predefined.json with 4 DummyPlugin connections:
    predefined.zip
  2. Delete config.json
  3. Start BN
  4. Close BN and start again

The automatically created project "DummySource" is present twice

  • Bug: Changing the port of a DummyPlugin connection results in broken connection
  1. Start DummyBuildServer with Ports 1111/1112
  2. Correctly configure BN to communicate with said ports
  3. Check that the connection works. It should
  4. Go into settings of BN
  5. Stop DummyBuildServer and restart at 1300/1400
  6. Change ports of the build and source control connections to 1300/1400 accordingly
  7. Close settings

Errors occur. Timeout Exceptions
image

  • Bug: Creating two projects for DummyBuildServer does not work
    I assume this never really worked previously so I think this is a WillNotFix

  • Bug: The original "Test connection" button doesn't seem to do anything

  • Bug: Type of connection is not persisted for TFS Plugin

  1. Create two connections with TFS Plugin one as source control the other as build
  2. Change any value so the connections themselves are persisted (see bug above)
  3. Close and reopen settings

Both connections are now configured as build provider

  • Bug: Test connection does not yield a result in TFS Plugin

  • Bug: Available Projects are not updated when adding values in TFS Plugin

  1. Create new connection
  2. Use Azure DevOps Plugin and enter valid values

The projects combobox is not updated. Only when settings are closed and reopened. Even then, in my test, it loaded endlessly

  • Bug: Switching between connection types resets the selected Plugintype
  1. Create new connection and select Azure DevOps Server
  2. Switch connection type from "Build" to "Source control"

The Azure DevOps Server selection is reset

  • Question: Whats the point of the collection tests within the DummyPlugin settings?
  • Question: Why are there two separate test buttons?

@TheSylence TheSylence added this to the 0.4.0 milestone Feb 25, 2020
@TheSylence TheSylence self-assigned this Feb 25, 2020
@TheSylence TheSylence linked an issue Feb 25, 2020 that may be closed by this pull request
@TheSylence TheSylence linked an issue Mar 6, 2020 that may be closed by this pull request
@TheSylence
Copy link
Copy Markdown
Member Author

Bug: Changing the port of a DummyPlugin connection results in broken connection

  1. Start DummyBuildServer with Ports 1111/1112
  2. Correctly configure BN to communicate with said ports
  3. Check that the connection works. It should
  4. Go into settings of BN
  5. Stop DummyBuildServer and restart at 1300/1400
  6. Change ports of the build and source control connections to 1300/1400 accordingly
  7. Close settings
  8. Errors occur. Timeout Exceptions

I suggest to rewrite the DummyBuildServer to a more robust solution and exclude these issues from this PR.

@BerndNK
Copy link
Copy Markdown
Contributor

BerndNK commented May 27, 2020

I suggest to rewrite the DummyBuildServer to a more robust solution and exclude these issues from this PR.

Works for me. Can you link the issue ID here?

@TheSylence
Copy link
Copy Markdown
Member Author

Sure: #170

# Conflicts:
#	.editorconfig
#	Plugins/BuildNotifications.Plugin.GitHub/BuildNotifications.Plugin.GitHub.csproj
#	Plugins/BuildNotifications.Plugin.Tfs/BuildNotifications.Plugin.Tfs.csproj
@TheSylence
Copy link
Copy Markdown
Member Author

Bug: Automatically created project for predefined connections is created twice

This is "working as currently intended". It searches for a project that contains ALL predefined connections and can't find one.

We already have #70 to address this. So I'd say we can ignore this for now.

@TheSylence TheSylence requested a review from BerndNK June 6, 2020 19:37
# Conflicts:
#	BuildNotifications.Core/Config/Configuration.cs
#	BuildNotifications.Core/Config/ConfigurationSerializer.cs
#	BuildNotifications.Core/CoreSetup.cs
#	BuildNotifications.Core/Pipeline/Pipeline.cs
#	BuildNotifications.Core/Pipeline/ProjectFactory.cs
#	BuildNotifications.Core/Plugin/PluginLoader.cs
#	BuildNotifications.Core/Utilities/Serializer.cs
#	BuildNotifications/AssemblyInfo.cs
#	BuildNotifications/BuildNotifications.csproj
#	BuildNotifications/Resources/Settings/PluginTypeToIconConverter.cs
#	BuildNotifications/ViewModel/MainViewModel.cs
#	Plugins/BuildNotifications.Plugin.Tfs/AssemblyInfo.cs
#	Plugins/BuildNotifications.Plugin.Tfs/TfsConfiguration.cs
#	Plugins/BuildNotifications.Plugin.Tfs/TfsConnectionPool.cs
#	Plugins/BuildNotifications.Plugin.Tfs/TfsPlugin.cs
#	Plugins/BuildNotifications.PluginInterfaces/Configuration/PasswordStringConverter.cs
# Conflicts:
#	Plugins/BuildNotifications.Plugin.Tfs/BuildNotifications.Plugin.Tfs.csproj
# Conflicts:
#	BuildNotifications/ViewModel/MainViewModel.cs
#	Plugins/BuildNotifications.Plugin.GitHub/BuildNotifications.Plugin.GitHub.csproj
#	Plugins/BuildNotifications.Plugin.Tfs/BuildNotifications.Plugin.Tfs.csproj
#	Plugins/BuildNotifications.PluginInterfaces/BuildNotifications.PluginInterfaces.csproj
@TheSylence TheSylence linked an issue Jun 10, 2020 that may be closed by this pull request
Comment thread BuildNotifications.Core/Config/ConfigurationBuilder.cs Outdated
# Conflicts:
#	BuildNotifications.Core.Tests/Utilities/SerializerTests.cs
#	BuildNotifications.Core/Text/Texts.de.resx
Comment thread BuildNotifications/ViewModel/Settings/Setup/SetupViewModel.cs
Co-authored-by: BerndNK <BerndKrupinski@gmail.com>
@TheSylence TheSylence merged commit 4e8354a into master Jun 10, 2020
@TheSylence TheSylence deleted the SettingsSystemRefactoring branch June 10, 2020 14:11
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.

Remove "Show PullRequest" setting Connection Settings are resetted Extend Predefined connections Refactor settings system

2 participants