From 6803e467825072bd5a0439e542d87846f894d038 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Wed, 18 Feb 2015 00:43:33 +0100 Subject: [PATCH] Fixed: Empty Sabnzbd category is now properly handled. But added UI validation to recommend adding a category. --- .../DownloadClient/DownloadClientModule.cs | 4 +- src/NzbDrone.Api/Indexers/IndexerModule.cs | 4 +- src/NzbDrone.Api/Metadata/MetadataModule.cs | 4 +- .../Notifications/NotificationModule.cs | 4 +- src/NzbDrone.Api/ProviderModuleBase.cs | 56 +++++++++++-------- .../Download/Clients/Deluge/DelugeSettings.cs | 2 +- .../Download/Clients/Nzbget/NzbgetSettings.cs | 5 +- .../Download/Clients/Sabnzbd/Sabnzbd.cs | 2 +- .../Clients/Sabnzbd/SabnzbdSettings.cs | 7 ++- .../Transmission/TransmissionSettings.cs | 2 +- .../Clients/uTorrent/UTorrentSettings.cs | 2 +- src/NzbDrone.Core/NzbDrone.Core.csproj | 4 +- ...gaugeValidator.cs => LanguageValidator.cs} | 4 +- .../Validation/NzbDroneValidationFailure.cs | 22 +++++++- .../Validation/NzbDroneValidationResult.cs | 55 ++++++++++++++++++ .../Validation/NzbDroneValidationState.cs | 16 ++++++ .../Validation/NzbDroneValidator.cs | 17 ++++++ .../Validation/RuleBuilderExtensions.cs | 7 ++- src/UI/Content/form.less | 14 +++++ src/UI/jQuery/jquery.validation.js | 30 ++++++---- 20 files changed, 207 insertions(+), 54 deletions(-) rename src/NzbDrone.Core/Validation/{LangaugeValidator.cs => LanguageValidator.cs} (81%) create mode 100644 src/NzbDrone.Core/Validation/NzbDroneValidationResult.cs create mode 100644 src/NzbDrone.Core/Validation/NzbDroneValidationState.cs create mode 100644 src/NzbDrone.Core/Validation/NzbDroneValidator.cs diff --git a/src/NzbDrone.Api/DownloadClient/DownloadClientModule.cs b/src/NzbDrone.Api/DownloadClient/DownloadClientModule.cs index cdef47a78..78d997739 100644 --- a/src/NzbDrone.Api/DownloadClient/DownloadClientModule.cs +++ b/src/NzbDrone.Api/DownloadClient/DownloadClientModule.cs @@ -9,10 +9,10 @@ public DownloadClientModule(IDownloadClientFactory downloadClientFactory) { } - protected override void Validate(DownloadClientDefinition definition) + protected override void Validate(DownloadClientDefinition definition, bool includeWarnings) { if (!definition.Enable) return; - base.Validate(definition); + base.Validate(definition, includeWarnings); } } } \ No newline at end of file diff --git a/src/NzbDrone.Api/Indexers/IndexerModule.cs b/src/NzbDrone.Api/Indexers/IndexerModule.cs index 521ce0bfa..0ad5f5e25 100644 --- a/src/NzbDrone.Api/Indexers/IndexerModule.cs +++ b/src/NzbDrone.Api/Indexers/IndexerModule.cs @@ -9,10 +9,10 @@ public IndexerModule(IndexerFactory indexerFactory) { } - protected override void Validate(IndexerDefinition definition) + protected override void Validate(IndexerDefinition definition, bool includeWarnings) { if (!definition.Enable) return; - base.Validate(definition); + base.Validate(definition, includeWarnings); } } } \ No newline at end of file diff --git a/src/NzbDrone.Api/Metadata/MetadataModule.cs b/src/NzbDrone.Api/Metadata/MetadataModule.cs index 89a6374b0..b831f1a49 100644 --- a/src/NzbDrone.Api/Metadata/MetadataModule.cs +++ b/src/NzbDrone.Api/Metadata/MetadataModule.cs @@ -9,10 +9,10 @@ public MetadataModule(IMetadataFactory metadataFactory) { } - protected override void Validate(MetadataDefinition definition) + protected override void Validate(MetadataDefinition definition, bool includeWarnings) { if (!definition.Enable) return; - base.Validate(definition); + base.Validate(definition, includeWarnings); } } } \ No newline at end of file diff --git a/src/NzbDrone.Api/Notifications/NotificationModule.cs b/src/NzbDrone.Api/Notifications/NotificationModule.cs index fb6130e2c..ec5d14d39 100644 --- a/src/NzbDrone.Api/Notifications/NotificationModule.cs +++ b/src/NzbDrone.Api/Notifications/NotificationModule.cs @@ -9,10 +9,10 @@ public NotificationModule(NotificationFactory notificationFactory) { } - protected override void Validate(NotificationDefinition definition) + protected override void Validate(NotificationDefinition definition, bool includeWarnings) { if (!definition.OnGrab && !definition.OnDownload) return; - base.Validate(definition); + base.Validate(definition, includeWarnings); } } } \ No newline at end of file diff --git a/src/NzbDrone.Api/ProviderModuleBase.cs b/src/NzbDrone.Api/ProviderModuleBase.cs index 7e2a95c5f..3d12e8c10 100644 --- a/src/NzbDrone.Api/ProviderModuleBase.cs +++ b/src/NzbDrone.Api/ProviderModuleBase.cs @@ -2,12 +2,14 @@ using System.Collections.Generic; using System.Linq; using FluentValidation; +using FluentValidation.Results; using Nancy; using NzbDrone.Api.ClientSchema; using NzbDrone.Api.Extensions; using NzbDrone.Api.Mapping; using NzbDrone.Common.Reflection; using NzbDrone.Core.ThingiProvider; +using NzbDrone.Core.Validation; using Omu.ValueInjecter; namespace NzbDrone.Api @@ -72,12 +74,9 @@ private List GetAll() private int CreateProvider(TProviderResource providerResource) { - var providerDefinition = GetDefinition(providerResource); + var providerDefinition = GetDefinition(providerResource, false); - if (providerDefinition.Enable) - { - Test(providerDefinition); - } + Test(providerDefinition, false); providerDefinition = _providerFactory.Create(providerDefinition); @@ -86,12 +85,14 @@ private int CreateProvider(TProviderResource providerResource) private void UpdateProvider(TProviderResource providerResource) { - var providerDefinition = GetDefinition(providerResource); + var providerDefinition = GetDefinition(providerResource, false); + + Test(providerDefinition, false); _providerFactory.Update(providerDefinition); } - private TProviderDefinition GetDefinition(TProviderResource providerResource) + private TProviderDefinition GetDefinition(TProviderResource providerResource, bool includeWarnings = false) { var definition = new TProviderDefinition(); @@ -105,7 +106,7 @@ private TProviderDefinition GetDefinition(TProviderResource providerResource) var configContract = ReflectionExtensions.CoreAssembly.FindTypeByName(definition.ConfigContract); definition.Settings = (IProviderConfig)SchemaBuilder.ReadFormSchema(providerResource.Fields, configContract, preset); - Validate(definition); + Validate(definition, includeWarnings); return definition; } @@ -149,31 +150,42 @@ private Response GetTemplates() private Response Test(TProviderResource providerResource) { - var providerDefinition = GetDefinition(providerResource); + var providerDefinition = GetDefinition(providerResource, true); - Test(providerDefinition); + Test(providerDefinition, true); return "{}"; } - private void Test(TProviderDefinition providerDefinition) + protected virtual void Validate(TProviderDefinition definition, bool includeWarnings) { - var result = _providerFactory.Test(providerDefinition); + var validationResult = definition.Settings.Validate(); + + VerifyValidationResult(validationResult, includeWarnings); + } + + protected virtual void Test(TProviderDefinition definition, bool includeWarnings) + { + if (!definition.Enable) return; + + var validationResult = _providerFactory.Test(definition); + + VerifyValidationResult(validationResult, includeWarnings); + } + + protected void VerifyValidationResult(ValidationResult validationResult, bool includeWarnings) + { + var result = new NzbDroneValidationResult(validationResult.Errors); + + if (includeWarnings && (!result.IsValid || result.HasWarnings)) + { + throw new ValidationException(result.Failures); + } if (!result.IsValid) { throw new ValidationException(result.Errors); } } - - protected virtual void Validate(TProviderDefinition definition) - { - var validationResult = definition.Settings.Validate(); - - if (!validationResult.IsValid) - { - throw new ValidationException(validationResult.Errors); - } - } } } diff --git a/src/NzbDrone.Core/Download/Clients/Deluge/DelugeSettings.cs b/src/NzbDrone.Core/Download/Clients/Deluge/DelugeSettings.cs index 97136d54f..3f3412f02 100644 --- a/src/NzbDrone.Core/Download/Clients/Deluge/DelugeSettings.cs +++ b/src/NzbDrone.Core/Download/Clients/Deluge/DelugeSettings.cs @@ -38,7 +38,7 @@ public DelugeSettings() [FieldDefinition(2, Label = "Password", Type = FieldType.Password)] public String Password { get; set; } - [FieldDefinition(3, Label = "Category", Type = FieldType.Textbox)] + [FieldDefinition(3, Label = "Category", Type = FieldType.Textbox, HelpText = "Adding a category specific to Sonarr avoids conflicts with unrelated downloads, but it's optional")] public String TvCategory { get; set; } [FieldDefinition(4, Label = "Recent Priority", Type = FieldType.Select, SelectOptions = typeof(DelugePriority), HelpText = "Priority to use when grabbing episodes that aired within the last 14 days")] diff --git a/src/NzbDrone.Core/Download/Clients/Nzbget/NzbgetSettings.cs b/src/NzbDrone.Core/Download/Clients/Nzbget/NzbgetSettings.cs index e6370a3d9..300dcbac5 100644 --- a/src/NzbDrone.Core/Download/Clients/Nzbget/NzbgetSettings.cs +++ b/src/NzbDrone.Core/Download/Clients/Nzbget/NzbgetSettings.cs @@ -3,6 +3,7 @@ using FluentValidation.Results; using NzbDrone.Core.Annotations; using NzbDrone.Core.ThingiProvider; +using NzbDrone.Core.Validation; using NzbDrone.Core.Validation.Paths; namespace NzbDrone.Core.Download.Clients.Nzbget @@ -18,6 +19,8 @@ public NzbgetSettingsValidator() RuleFor(c => c.TvCategory).NotEmpty().When(c => !String.IsNullOrWhiteSpace(c.TvCategoryLocalPath)); RuleFor(c => c.TvCategoryLocalPath).IsValidPath().When(c => !String.IsNullOrWhiteSpace(c.TvCategoryLocalPath)); + + RuleFor(c => c.TvCategory).NotEmpty().WithMessage("A category is recommended").AsWarning(); } } @@ -46,7 +49,7 @@ public NzbgetSettings() [FieldDefinition(3, Label = "Password", Type = FieldType.Password)] public String Password { get; set; } - [FieldDefinition(4, Label = "Category", Type = FieldType.Textbox)] + [FieldDefinition(4, Label = "Category", Type = FieldType.Textbox, HelpText = "Adding a category specific to Sonarr avoids conflicts with unrelated downloads, but it's optional")] public String TvCategory { get; set; } // TODO: Remove around January 2015, this setting was superceded by the RemotePathMappingService, but has to remain for a while to properly migrate. diff --git a/src/NzbDrone.Core/Download/Clients/Sabnzbd/Sabnzbd.cs b/src/NzbDrone.Core/Download/Clients/Sabnzbd/Sabnzbd.cs index d69cf8446..e90fdbaa0 100644 --- a/src/NzbDrone.Core/Download/Clients/Sabnzbd/Sabnzbd.cs +++ b/src/NzbDrone.Core/Download/Clients/Sabnzbd/Sabnzbd.cs @@ -181,7 +181,7 @@ public override IEnumerable GetItems() foreach (var downloadClientItem in GetQueue().Concat(GetHistory())) { - if (downloadClientItem.Category == Settings.TvCategory) + if (downloadClientItem.Category == Settings.TvCategory || downloadClientItem.Category == "*" && Settings.TvCategory.IsNullOrWhiteSpace()) { yield return downloadClientItem; } diff --git a/src/NzbDrone.Core/Download/Clients/Sabnzbd/SabnzbdSettings.cs b/src/NzbDrone.Core/Download/Clients/Sabnzbd/SabnzbdSettings.cs index da726299f..b360be487 100644 --- a/src/NzbDrone.Core/Download/Clients/Sabnzbd/SabnzbdSettings.cs +++ b/src/NzbDrone.Core/Download/Clients/Sabnzbd/SabnzbdSettings.cs @@ -3,6 +3,7 @@ using FluentValidation.Results; using NzbDrone.Core.Annotations; using NzbDrone.Core.ThingiProvider; +using NzbDrone.Core.Validation; namespace NzbDrone.Core.Download.Clients.Sabnzbd { @@ -24,6 +25,10 @@ public SabnzbdSettingsValidator() RuleFor(c => c.Password).NotEmpty() .WithMessage("Password is required when API key is not configured") .When(c => String.IsNullOrWhiteSpace(c.ApiKey)); + + RuleFor(c => c.TvCategory).NotEmpty() + .WithMessage("A category is recommended") + .AsWarning(); } } @@ -55,7 +60,7 @@ public SabnzbdSettings() [FieldDefinition(4, Label = "Password", Type = FieldType.Password)] public String Password { get; set; } - [FieldDefinition(5, Label = "Category", Type = FieldType.Textbox)] + [FieldDefinition(5, Label = "Category", Type = FieldType.Textbox, HelpText = "Adding a category specific to Sonarr avoids conflicts with unrelated downloads, but it's optional")] public String TvCategory { get; set; } // TODO: Remove around January 2015, this setting was superceded by the RemotePathMappingService, but has to remain for a while to properly migrate. diff --git a/src/NzbDrone.Core/Download/Clients/Transmission/TransmissionSettings.cs b/src/NzbDrone.Core/Download/Clients/Transmission/TransmissionSettings.cs index 158f7e867..f7becac64 100644 --- a/src/NzbDrone.Core/Download/Clients/Transmission/TransmissionSettings.cs +++ b/src/NzbDrone.Core/Download/Clients/Transmission/TransmissionSettings.cs @@ -42,7 +42,7 @@ public TransmissionSettings() [FieldDefinition(4, Label = "Password", Type = FieldType.Password)] public String Password { get; set; } - [FieldDefinition(5, Label = "Category", Type = FieldType.Textbox)] + [FieldDefinition(5, Label = "Category", Type = FieldType.Textbox, HelpText = "Adding a category specific to Sonarr avoids conflicts with unrelated downloads, but it's optional. Creates a .[category] subdirectory in the output directory.")] public String TvCategory { get; set; } [FieldDefinition(6, Label = "Recent Priority", Type = FieldType.Select, SelectOptions = typeof(TransmissionPriority), HelpText = "Priority to use when grabbing episodes that aired within the last 14 days")] diff --git a/src/NzbDrone.Core/Download/Clients/uTorrent/UTorrentSettings.cs b/src/NzbDrone.Core/Download/Clients/uTorrent/UTorrentSettings.cs index f88821682..496939f89 100644 --- a/src/NzbDrone.Core/Download/Clients/uTorrent/UTorrentSettings.cs +++ b/src/NzbDrone.Core/Download/Clients/uTorrent/UTorrentSettings.cs @@ -39,7 +39,7 @@ public UTorrentSettings() [FieldDefinition(3, Label = "Password", Type = FieldType.Password)] public String Password { get; set; } - [FieldDefinition(4, Label = "Category", Type = FieldType.Textbox)] + [FieldDefinition(4, Label = "Category", Type = FieldType.Textbox, HelpText = "Adding a category specific to Sonarr avoids conflicts with unrelated downloads, but it's optional")] public String TvCategory { get; set; } [FieldDefinition(5, Label = "Recent Priority", Type = FieldType.Select, SelectOptions = typeof(UTorrentPriority), HelpText = "Priority to use when grabbing episodes that aired within the last 14 days")] diff --git a/src/NzbDrone.Core/NzbDrone.Core.csproj b/src/NzbDrone.Core/NzbDrone.Core.csproj index c8b4b24d5..017edcc36 100644 --- a/src/NzbDrone.Core/NzbDrone.Core.csproj +++ b/src/NzbDrone.Core/NzbDrone.Core.csproj @@ -869,8 +869,10 @@ - + + + diff --git a/src/NzbDrone.Core/Validation/LangaugeValidator.cs b/src/NzbDrone.Core/Validation/LanguageValidator.cs similarity index 81% rename from src/NzbDrone.Core/Validation/LangaugeValidator.cs rename to src/NzbDrone.Core/Validation/LanguageValidator.cs index 678245f36..9edfc9085 100644 --- a/src/NzbDrone.Core/Validation/LangaugeValidator.cs +++ b/src/NzbDrone.Core/Validation/LanguageValidator.cs @@ -2,9 +2,9 @@ namespace NzbDrone.Core.Validation { - public class LangaugeValidator : PropertyValidator + public class LanguageValidator : PropertyValidator { - public LangaugeValidator() + public LanguageValidator() : base("Unknown Language") { } diff --git a/src/NzbDrone.Core/Validation/NzbDroneValidationFailure.cs b/src/NzbDrone.Core/Validation/NzbDroneValidationFailure.cs index ef2ee9c48..92c6835b3 100644 --- a/src/NzbDrone.Core/Validation/NzbDroneValidationFailure.cs +++ b/src/NzbDrone.Core/Validation/NzbDroneValidationFailure.cs @@ -5,13 +5,29 @@ namespace NzbDrone.Core.Validation { public class NzbDroneValidationFailure : ValidationFailure { - public String DetailedDescription { get; set; } - public String InfoLink { get; set; } + public bool IsWarning { get; set; } + public string DetailedDescription { get; set; } + public string InfoLink { get; set; } - public NzbDroneValidationFailure(String propertyName, String error) + public NzbDroneValidationFailure(string propertyName, string error) : base(propertyName, error) { } + + public NzbDroneValidationFailure(string propertyName, string error, object attemptedValue) + : base(propertyName, error, attemptedValue) + { + + } + + public NzbDroneValidationFailure(ValidationFailure validationFailure) + : base(validationFailure.PropertyName, validationFailure.ErrorMessage, validationFailure.AttemptedValue) + { + CustomState = validationFailure.CustomState; + var state = validationFailure.CustomState as NzbDroneValidationState; + + IsWarning = state != null && state.IsWarning; + } } } diff --git a/src/NzbDrone.Core/Validation/NzbDroneValidationResult.cs b/src/NzbDrone.Core/Validation/NzbDroneValidationResult.cs new file mode 100644 index 000000000..804e8e147 --- /dev/null +++ b/src/NzbDrone.Core/Validation/NzbDroneValidationResult.cs @@ -0,0 +1,55 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using FluentValidation; +using FluentValidation.Results; + +namespace NzbDrone.Core.Validation +{ + public class NzbDroneValidationResult : ValidationResult + { + public NzbDroneValidationResult() + { + } + + public NzbDroneValidationResult(IEnumerable failures) + { + var errors = new List(); + var warnings = new List(); + + foreach (var failureBase in failures) + { + var failure = failureBase as NzbDroneValidationFailure; + if (failure == null) + { + failure = new NzbDroneValidationFailure(failureBase); + } + if (failure.IsWarning) + { + warnings.Add(failure); + } + else + { + errors.Add(failure); + } + } + + Failures = errors.Concat(warnings).ToList(); + Errors = errors; + errors.ForEach(base.Errors.Add); + Warnings = warnings; + } + + public IList Failures { get; private set; } + + public new IList Errors { get; private set; } + + public IList Warnings { get; private set; } + + public virtual bool HasWarnings + { + get { return Warnings.Any(); } + } + } +} \ No newline at end of file diff --git a/src/NzbDrone.Core/Validation/NzbDroneValidationState.cs b/src/NzbDrone.Core/Validation/NzbDroneValidationState.cs new file mode 100644 index 000000000..4ed4d2990 --- /dev/null +++ b/src/NzbDrone.Core/Validation/NzbDroneValidationState.cs @@ -0,0 +1,16 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using FluentValidation; +using FluentValidation.Results; + +namespace NzbDrone.Core.Validation +{ + public class NzbDroneValidationState + { + public static NzbDroneValidationState Warning = new NzbDroneValidationState { IsWarning = true }; + + public bool IsWarning { get; set; } + } +} diff --git a/src/NzbDrone.Core/Validation/NzbDroneValidator.cs b/src/NzbDrone.Core/Validation/NzbDroneValidator.cs new file mode 100644 index 000000000..40e6348ae --- /dev/null +++ b/src/NzbDrone.Core/Validation/NzbDroneValidator.cs @@ -0,0 +1,17 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using FluentValidation; +using FluentValidation.Results; + +namespace NzbDrone.Core.Validation +{ + public abstract class NzbDroneValidator : AbstractValidator + { + public override ValidationResult Validate(T instance) + { + return new NzbDroneValidationResult(base.Validate(instance).Errors); + } + } +} diff --git a/src/NzbDrone.Core/Validation/RuleBuilderExtensions.cs b/src/NzbDrone.Core/Validation/RuleBuilderExtensions.cs index b0fb71377..c6c4aeb5b 100644 --- a/src/NzbDrone.Core/Validation/RuleBuilderExtensions.cs +++ b/src/NzbDrone.Core/Validation/RuleBuilderExtensions.cs @@ -35,7 +35,12 @@ public static IRuleBuilderOptions ValidPort(this IRuleBuilder public static IRuleBuilderOptions ValidLanguage(this IRuleBuilder ruleBuilder) { - return ruleBuilder.SetValidator(new LangaugeValidator()); + return ruleBuilder.SetValidator(new LanguageValidator()); + } + + public static IRuleBuilderOptions AsWarning(this IRuleBuilderOptions ruleBuilder) + { + return ruleBuilder.WithState(v => NzbDroneValidationState.Warning); } } } \ No newline at end of file diff --git a/src/UI/Content/form.less b/src/UI/Content/form.less index 65544e690..28474c962 100644 --- a/src/UI/Content/form.less +++ b/src/UI/Content/form.less @@ -102,6 +102,20 @@ h3 { } } +.has-warning { + .help-inline { + color: orange; + margin-left: 0px; + } +} + +.validation-warning { + i { + text-decoration: none; + color: orange; + } +} + // Tooltips .help-inline-checkbox, .help-inline { diff --git a/src/UI/jQuery/jquery.validation.js b/src/UI/jQuery/jquery.validation.js index ba66c3651..af8efe3a4 100644 --- a/src/UI/jQuery/jquery.validation.js +++ b/src/UI/jQuery/jquery.validation.js @@ -37,16 +37,21 @@ module.exports = function() { } else { var inputGroup = formGroup.find('.input-group'); - if (inputGroup.length === 0) { - formGroup.append('' + errorMessage + ''); - } + var validationClass = error.isWarning ? 'validation-warning' : 'validation-error'; + if (inputGroup.length === 0) { + formGroup.append('{1}'.format(validationClass, errorMessage)); + } else { - inputGroup.parent().append('' + errorMessage + ''); + inputGroup.parent().append('{1}'.format(validationClass, errorMessage)); } } - formGroup.addClass('has-error'); + if (error.isWarning) { + formGroup.addClass('has-warning'); + } else { + formGroup.addClass('has-error'); + } return formGroup.find('.help-inline').text(); }; @@ -59,20 +64,23 @@ module.exports = function() { var errorMessage = this.formatErrorMessage(error); - if (this.find('.modal-body')) { - this.find('.modal-body').prepend('
' + errorMessage + '
'); + var target = this.find('.modal-body'); + if (!target.length) { + target = this; } - else { - this.prepend('
' + errorMessage + '
'); - } + var validationClass = error.isWarning ? 'alert alert-warning validation-warning' : 'alert alert-danger validation-error'; + + target.prepend('
{1}
'.format(validationClass, errorMessage)); }; $.fn.removeAllErrors = function() { this.find('.has-error').removeClass('has-error'); + this.find('.has-warning').removeClass('has-warning'); this.find('.error').removeClass('error'); - this.find('.validation-errors').removeClass('alert').removeClass('alert-danger').html(''); + this.find('.validation-errors').removeClass('alert').removeClass('alert-danger').removeClass('alert-warning').html(''); this.find('.validation-error').remove(); + this.find('.validation-warning').remove(); return this.find('.help-inline.error-message').remove(); };