Fix missing per-instance failed import strikes setting (#837)

This commit is contained in:
Flaminel authored and GitHub committed 2026-10-14 16:02:36 +03:00
1 parent 87b7a621dd
commit a4a4ccee1b
9 files changed
+464 -11

No files matched your search

@@ -3,6 +3,7 @@ using Cleanuparr.Api.Features.Arr.Contracts.Requests;
using Cleanuparr.Api.Features.Arr.Controllers;
using Cleanuparr.Api.Tests.TestHelpers;
using Cleanuparr.Domain.Enums;
using Cleanuparr.Domain.Exceptions;
using Cleanuparr.Infrastructure.Events.Interfaces;
using Cleanuparr.Api.Features.Arr.Contracts.Responses;
using Cleanuparr.Infrastructure.Features.Arr.Interfaces;
@@ -103,17 +104,35 @@ public class ArrConfigControllerTests : IDisposable
saved.FailedImportMaxStrikes.ShouldBe((short)7);
}
[Fact]
public async Task UpdateSonarrConfig_DefaultStrikes_PassesThrough()
[Theory]
[InlineData(-1)]
[InlineData(0)]
[InlineData(3)]
public async Task UpdateSonarrConfig_ValidStrikes_PersistsValue(short strikes)
{
// ArrConfig.Validate is currently a no-op; -1 (the default disabled value) is accepted
var request = new UpdateArrConfigRequest { FailedImportMaxStrikes = -1 };
// Arrange
var request = new UpdateArrConfigRequest { FailedImportMaxStrikes = strikes };
// Act
var result = await _controller.UpdateSonarrConfig(request);
// Assert
result.ShouldBeOfType<OkObjectResult>();
var saved = await _dataContext.ArrConfigs.AsNoTracking().FirstAsync(c => c.Type == InstanceType.Sonarr);
saved.FailedImportMaxStrikes.ShouldBe(strikes);
}
[Theory]
[InlineData(-2)]
[InlineData(1)]
[InlineData(2)]
public async Task UpdateSonarrConfig_InvalidStrikes_ThrowsValidationException(short strikes)
{
// Arrange
var request = new UpdateArrConfigRequest { FailedImportMaxStrikes = strikes };
// Act & Assert
await Should.ThrowAsync<ValidationException>(() => _controller.UpdateSonarrConfig(request));
}
#endregion
@@ -0,0 +1,68 @@
using Cleanuparr.Domain.Enums;
using Cleanuparr.Persistence.Models.Configuration.Arr;
using Shouldly;
using Xunit;
using ValidationException = Cleanuparr.Domain.Exceptions.ValidationException;
namespace Cleanuparr.Persistence.Tests.Models.Configuration.Arr;
public sealed class ArrConfigTests
{
#region Validate - Valid Values
[Theory]
[InlineData((short)-1)]
[InlineData((short)0)]
[InlineData((short)3)]
[InlineData((short)100)]
public void Validate_WithValidStrikes_DoesNotThrow(short strikes)
{
ArrConfig config = new()
{
Type = InstanceType.Sonarr,
FailedImportMaxStrikes = strikes,
};
Should.NotThrow(() => config.Validate());
}
#endregion
#region Validate - Below -1
[Theory]
[InlineData((short)-2)]
[InlineData((short)-100)]
public void Validate_WithStrikesBelowMinusOne_ThrowsValidationException(short strikes)
{
ArrConfig config = new()
{
Type = InstanceType.Sonarr,
FailedImportMaxStrikes = strikes,
};
ValidationException exception = Should.Throw<ValidationException>(() => config.Validate());
exception.Message.ShouldBe("Failed import max strikes must be -1 or greater");
}
#endregion
#region Validate - Between 1 and 2
[Theory]
[InlineData((short)1)]
[InlineData((short)2)]
public void Validate_WithStrikesBetween1And2_ThrowsValidationException(short strikes)
{
ArrConfig config = new()
{
Type = InstanceType.Sonarr,
FailedImportMaxStrikes = strikes,
};
ValidationException exception = Should.Throw<ValidationException>(() => config.Validate());
exception.Message.ShouldBe("The minimum value for failed import max strikes must be 3");
}
#endregion
}
@@ -1,6 +1,7 @@
using System.ComponentModel.DataAnnotations;
using System.ComponentModel.DataAnnotations.Schema;
using Cleanuparr.Domain.Enums;
using ValidationException = Cleanuparr.Domain.Exceptions.ValidationException;
namespace Cleanuparr.Persistence.Models.Configuration.Arr;
@@ -9,14 +10,23 @@ public class ArrConfig : IConfig
[Key]
[DatabaseGenerated(DatabaseGeneratedOption.Identity)]
public Guid Id { get; set; } = Guid.NewGuid();
public required InstanceType Type { get; set; }
public short FailedImportMaxStrikes { get; set; } = -1;
public List<ArrInstance> Instances { get; set; } = [];
public void Validate()
{
if (FailedImportMaxStrikes < -1)
{
throw new ValidationException("Failed import max strikes must be -1 or greater");
}
if (FailedImportMaxStrikes is > 0 and < 3)
{
throw new ValidationException("The minimum value for failed import max strikes must be 3");
}
}
}
@@ -199,6 +199,8 @@ export class DocumentationService {
'externalUrl': 'external-url',
'apiKey': 'api-key',
'version': 'api-version',
'failedImportOverride': 'failed-import-max-strikes',
'failedImportMaxStrikes': 'failed-import-max-strikes',
},
'account': {
'oidcEnabled': 'enable-oidc',
@@ -18,6 +18,27 @@
<app-loading-state message="Loading settings..." />
} @else if (!loader.loading()) {
<div class="settings-form">
<app-card header="Failed Import">
<div class="form-stack">
<app-toggle label="Override global max strikes" [formField]="failedImportForm.override"
hint="Use a different failed import max strikes value than the queue cleaner setting"
helpKey="arr:failedImportOverride" />
@if (failedImportForm.override().value()) {
<app-number-input label="Max Strikes" [formField]="failedImportForm.maxStrikes"
hint="Number of strikes before action is taken (0 to disable for this app, min 3 to enable)"
[error]="failedImportForm.maxStrikes().errors()[0]?.message"
helpKey="arr:failedImportMaxStrikes" />
}
</div>
<div class="form-actions">
<app-button variant="primary" [glowing]="failedImportDirty()" [loading]="failedImportSaving()"
[disabled]="failedImportSaving() || failedImportSaved() || failedImportHasErrors() || !failedImportDirty()"
(clicked)="saveFailedImport()">
{{ failedImportSaved() ? 'Saved!' : 'Save Settings' }}
</app-button>
</div>
</app-card>
<app-card>
<div class="instances-header">
<h3 class="instances-header__title">Instances</h3>
@@ -5,6 +5,8 @@
.settings-form { @include settings-form; }
.modal-form { @include modal-form; }
.form-stack { @include form-stack; }
.form-actions { @include form-actions; }
.instances-header {
@include list-header;
@@ -34,6 +34,7 @@ const CONFIG: ArrConfig = {
function createApi(config: ArrConfig = CONFIG) {
return {
getConfig: vi.fn(() => of(config)),
updateConfig: vi.fn(() => of(undefined)),
createInstance: vi.fn(() => of(INSTANCE)),
updateInstance: vi.fn(() => of(INSTANCE)),
deleteInstance: vi.fn(() => of(undefined)),
@@ -441,4 +442,234 @@ describe('ArrSettingsComponent', () => {
expect(toast.error).toHaveBeenCalledWith('Delete error');
});
it('seeds the override toggle off and hides the number input when the config value is -1', () => {
const { fixture, component } = setup('sonarr', createApi({ failedImportMaxStrikes: -1, instances: [] }));
expect(component.failedImportModel()).toEqual({ override: false, maxStrikes: 3 });
expect(fixture.nativeElement.querySelector('app-number-input')).toBeNull();
expect(component.hasPendingChanges()).toBe(false);
});
it('seeds the override toggle on with the configured value', () => {
const { component } = setup('sonarr', createApi({ failedImportMaxStrikes: 3, instances: [] }));
expect(component.failedImportModel()).toEqual({ override: true, maxStrikes: 3 });
expect(component.hasPendingChanges()).toBe(false);
});
it('enables the override, sets a value and saves it', () => {
const { fixture, component, api, toast } = setup('sonarr', createApi({ failedImportMaxStrikes: -1, instances: [] }));
component.failedImportForm.override().value.set(true);
fixture.detectChanges();
component.failedImportForm.maxStrikes().value.set(5);
fixture.detectChanges();
expect(component.hasPendingChanges()).toBe(true);
component.saveFailedImport();
fixture.detectChanges();
expect(api.updateConfig).toHaveBeenCalledWith('sonarr', { failedImportMaxStrikes: 5 });
expect(toast.success).toHaveBeenCalledWith('Failed import settings saved');
expect(component.hasPendingChanges()).toBe(false);
});
it('disabling the override sends -1', () => {
const { fixture, component, api } = setup('sonarr', createApi({ failedImportMaxStrikes: 5, instances: [] }));
component.failedImportForm.override().value.set(false);
fixture.detectChanges();
component.saveFailedImport();
fixture.detectChanges();
expect(api.updateConfig).toHaveBeenCalledWith('sonarr', { failedImportMaxStrikes: -1 });
});
it('rejects 1 and 2 strikes, accepts 0, and disables save while invalid', () => {
const { fixture, component } = setup('sonarr', createApi({ failedImportMaxStrikes: -1, instances: [] }));
component.failedImportForm.override().value.set(true);
fixture.detectChanges();
component.failedImportForm.maxStrikes().value.set(1);
fixture.detectChanges();
expect(component.failedImportHasErrors()).toBe(true);
expect(component.failedImportForm.maxStrikes().errors()[0].message).toBe('Minimum value is 3');
component.failedImportForm.maxStrikes().value.set(2);
fixture.detectChanges();
expect(component.failedImportHasErrors()).toBe(true);
component.failedImportForm.maxStrikes().value.set(0);
fixture.detectChanges();
expect(component.failedImportHasErrors()).toBe(false);
component.failedImportForm.maxStrikes().value.set(1);
fixture.detectChanges();
const saveButton = fixture.nativeElement.querySelector('.form-actions button') as HTMLButtonElement;
expect(saveButton.disabled).toBe(true);
});
it('rejects fractional failed import strikes', () => {
const { fixture, component, api } = setup('sonarr', createApi({ failedImportMaxStrikes: -1, instances: [] }));
component.failedImportForm.override().value.set(true);
fixture.detectChanges();
component.failedImportForm.maxStrikes().value.set(3.5);
fixture.detectChanges();
expect(component.failedImportForm.maxStrikes().errors()[0].message).toBe('Value must be a whole number');
expect(component.failedImportHasErrors()).toBe(true);
component.saveFailedImport();
fixture.detectChanges();
expect(api.updateConfig).not.toHaveBeenCalled();
});
it.each([
[null, 'This field is required'],
[-5, 'Value cannot be negative'],
[5001, 'Value cannot exceed 5000'],
])('rejects a failed import maxStrikes of %s with "%s"', (maxStrikes, message) => {
const { fixture, component } = setup('sonarr', createApi({ failedImportMaxStrikes: -1, instances: [] }));
component.failedImportForm.override().value.set(true);
fixture.detectChanges();
component.failedImportForm.maxStrikes().value.set(maxStrikes);
fixture.detectChanges();
expect(component.failedImportForm.maxStrikes().errors()[0].message).toBe(message);
});
it('accepts a failed import maxStrikes of 5000', () => {
const { fixture, component } = setup('sonarr', createApi({ failedImportMaxStrikes: -1, instances: [] }));
component.failedImportForm.override().value.set(true);
fixture.detectChanges();
component.failedImportForm.maxStrikes().value.set(5000);
fixture.detectChanges();
expect(component.failedImportForm.maxStrikes().errors().length).toBe(0);
});
it('ignores an invalid failed import maxStrikes while the override is off', () => {
const { fixture, component } = setup('sonarr', createApi({ failedImportMaxStrikes: -1, instances: [] }));
component.failedImportForm.maxStrikes().value.set(1);
fixture.detectChanges();
expect(component.failedImportForm.maxStrikes().errors().length).toBe(0);
expect(component.failedImportHasErrors()).toBe(false);
});
it('renders the failed import maxStrikes error message in the number input', () => {
const { fixture, component } = setup('sonarr', createApi({ failedImportMaxStrikes: -1, instances: [] }));
component.failedImportForm.override().value.set(true);
fixture.detectChanges();
component.failedImportForm.maxStrikes().value.set(1);
fixture.detectChanges();
expect(text(fixture, '.number-error')).toEqual(['Minimum value is 3']);
});
it('stays clean when the override is toggled back off after editing the hidden value', () => {
const { fixture, component } = setup('sonarr', createApi({ failedImportMaxStrikes: -1, instances: [] }));
component.failedImportForm.override().value.set(true);
fixture.detectChanges();
component.failedImportForm.maxStrikes().value.set(9999);
fixture.detectChanges();
component.failedImportForm.override().value.set(false);
fixture.detectChanges();
expect(component.hasPendingChanges()).toBe(false);
expect(component.failedImportHasErrors()).toBe(false);
});
it('toasts the error when saving the failed import override fails', () => {
const api = createApi({ failedImportMaxStrikes: -1, instances: [] });
api.updateConfig.mockReturnValue(throwError(() => new ApiError('Save failed')));
const { fixture, component, toast } = setup('sonarr', api);
component.failedImportForm.override().value.set(true);
fixture.detectChanges();
component.saveFailedImport();
fixture.detectChanges();
expect(toast.error).toHaveBeenCalledWith('Save failed');
expect(component.failedImportSaving()).toBe(false);
});
it('keeps unsaved failed import edits when an instance is saved', () => {
const { fixture, component, api } = setup('sonarr');
api.getConfig.mockReturnValue(of({ ...CONFIG }));
component.failedImportForm.maxStrikes().value.set(9);
fixture.detectChanges();
component.openEditModal(INSTANCE);
fixture.detectChanges();
component.saveInstance();
fixture.detectChanges();
expect(api.getConfig).toHaveBeenCalledTimes(2);
expect(component.failedImportModel().maxStrikes).toBe(9);
expect(component.hasPendingChanges()).toBe(true);
});
it('keeps unsaved failed import edits when an instance is deleted', async () => {
const { fixture, component, api, confirm } = setup('sonarr');
api.getConfig.mockReturnValue(of({ ...CONFIG }));
component.failedImportForm.maxStrikes().value.set(9);
fixture.detectChanges();
const accepted = component.deleteInstance(INSTANCE);
confirm.accept();
await accepted;
fixture.detectChanges();
expect(api.getConfig).toHaveBeenCalledTimes(2);
expect(component.failedImportModel().maxStrikes).toBe(9);
expect(component.hasPendingChanges()).toBe(true);
});
it('reseeds the failed import settings on reload when there are no unsaved edits', () => {
const api = createApi();
const { fixture, component } = setup('sonarr', api);
api.getConfig.mockReturnValueOnce(of({ ...CONFIG, failedImportMaxStrikes: 7 }));
component.openEditModal(INSTANCE);
fixture.detectChanges();
component.saveInstance();
fixture.detectChanges();
expect(component.failedImportModel().maxStrikes).toBe(7);
expect(component.hasPendingChanges()).toBe(false);
});
it('reseeds the failed import settings when the arr type changes despite unsaved edits', () => {
const { fixture, component, api } = setup('sonarr');
component.failedImportForm.maxStrikes().value.set(9);
fixture.detectChanges();
api.getConfig.mockReturnValue(of({ ...CONFIG, failedImportMaxStrikes: -1 }));
fixture.componentRef.setInput('type', 'radarr');
fixture.detectChanges();
expect(api.getConfig).toHaveBeenLastCalledWith('radarr');
expect(component.failedImportModel()).toEqual({ override: false, maxStrikes: 3 });
expect(component.hasPendingChanges()).toBe(false);
});
});
@@ -1,8 +1,8 @@
import { Component, ChangeDetectionStrategy, inject, signal, input, computed, effect, untracked } from '@angular/core';
import { form, required, FormField } from '@angular/forms/signals';
import { form, required, validate, FormField } from '@angular/forms/signals';
import { PageHeaderComponent } from '@layout/page-header/page-header.component';
import {
CardComponent, ButtonComponent, InputComponent, ToggleComponent,
CardComponent, ButtonComponent, InputComponent, ToggleComponent, NumberInputComponent,
SelectComponent, ModalComponent, EmptyStateComponent, BadgeComponent, LoadingStateComponent,
type SelectOption,
} from '@ui';
@@ -13,7 +13,8 @@ import { ConfirmService } from '@core/services/confirm.service';
import { ArrInstance, CreateArrInstanceDto, TestArrInstanceRequest } from '@shared/models/arr-config.model';
import { ArrType } from '@shared/models/enums';
import { HasPendingChanges } from '@core/guards/pending-changes.guard';
import { createSettingsResource } from '@shared/utils/settings-resource.util';
import { createSettingsResource, saveSettings } from '@shared/utils/settings-resource.util';
import { createDirtyTracker } from '@shared/utils/dirty-tracker.util';
const ARR_VERSION_OPTIONS: Record<string, SelectOption[]> = {
sonarr: [{ label: 'v4', value: 4 }],
@@ -46,12 +47,17 @@ interface ArrInstanceFormModel {
enabled: boolean;
}
interface FailedImportFormModel {
override: boolean;
maxStrikes: number | null;
}
@Component({
selector: 'app-arr-settings',
standalone: true,
imports: [
PageHeaderComponent, CardComponent, ButtonComponent, InputComponent,
ToggleComponent, SelectComponent, ModalComponent, EmptyStateComponent,
ToggleComponent, NumberInputComponent, SelectComponent, ModalComponent, EmptyStateComponent,
BadgeComponent, LoadingStateComponent, FormField,
],
templateUrl: './arr-settings.component.html',
@@ -90,6 +96,43 @@ export class ArrSettingsComponent implements HasPendingChanges {
this.configResource.hasValue() ? (this.configResource.value().instances ?? []) : [],
);
// Failed import override
readonly failedImportSaving = signal(false);
readonly failedImportSaved = signal(false);
readonly failedImportModel = signal<FailedImportFormModel>({ override: false, maxStrikes: 3 });
readonly failedImportForm = form(this.failedImportModel, (p) => {
validate(p.maxStrikes, ({ value, valueOf }) => {
if (!valueOf(p.override)) {
return undefined;
}
if (value() === null) {
return { kind: 'required', message: 'This field is required' };
}
if (!Number.isInteger(value())) {
return { kind: 'integer', message: 'Value must be a whole number' };
}
if (value()! < 0) {
return { kind: 'min', message: 'Value cannot be negative' };
}
if (value()! > 0 && value()! < 3) {
return { kind: 'min', message: 'Minimum value is 3' };
}
if (value()! > 5000) {
return { kind: 'max', message: 'Value cannot exceed 5000' };
}
return undefined;
});
});
readonly failedImportHasErrors = computed(() => this.failedImportForm().invalid());
private readonly failedImportDirtyTracker = createDirtyTracker(
this.failedImportModel,
(m) => (m.override ? m.maxStrikes : -1),
);
readonly failedImportDirty = this.failedImportDirtyTracker.dirty;
/** Arr type the failed import model was last seeded from; unsaved edits survive only same-type reloads. */
private seededType: string | null = null;
// Modal state
readonly modalVisible = signal(false);
readonly editingInstance = signal<ArrInstance | null>(null);
@@ -118,6 +161,26 @@ export class ArrSettingsComponent implements HasPendingChanges {
untracked(() => this.instanceModel.update(m => ({ ...m, version: options[0].value as number })));
}
});
effect(() => {
if (!this.configResource.hasValue()) {
return;
}
const config = this.configResource.value();
untracked(() => {
const type = this.type();
if (this.seededType === type && this.failedImportDirty()) {
return;
}
this.seededType = type;
this.failedImportModel.set(
config.failedImportMaxStrikes >= 0
? { override: true, maxStrikes: config.failedImportMaxStrikes }
: { override: false, maxStrikes: 3 },
);
this.failedImportDirtyTracker.markSaved();
});
});
}
retry(): void {
@@ -224,7 +287,24 @@ export class ArrSettingsComponent implements HasPendingChanges {
});
}
saveFailedImport(): void {
if (this.failedImportForm().invalid()) {
return;
}
const m = this.failedImportModel();
const maxStrikes = m.override ? (m.maxStrikes ?? 3) : -1;
saveSettings({
request: this.api.updateConfig(this.type() as ArrType, { failedImportMaxStrikes: maxStrikes }),
toast: this.toast,
saving: this.failedImportSaving,
saved: this.failedImportSaved,
successMessage: 'Failed import settings saved',
onSaved: () => this.failedImportDirtyTracker.markSaved(m),
});
}
hasPendingChanges(): boolean {
return this.modalDirty();
return this.modalDirty() || this.failedImportDirty();
}
}
+20
View File
@@ -106,6 +106,26 @@ The API version for this *arr application. Select the version that matches your
</div>
<div className={styles.section}>
<SectionTitle>
Failed Import
</SectionTitle>
<ConfigSection
title="Failed Import Max Strikes"
>
Per-app override of the Queue Cleaner's [Failed Import Max Strikes](../queue-cleaner#failed-import-max-strikes) setting.
- **Off**: this app uses the Queue Cleaner's global value.
- **On, 0**: failed import strikes are disabled for this app.
- **On, 3 or higher**: this app uses its own max strikes value instead of the global one.
</ConfigSection>
</div>
</div>
## LazyLibrarian