Allow per-user Radarr and Sonarr quality profile selection#5449
Open
herbalizer404 wants to merge 5 commits into
Open
Allow per-user Radarr and Sonarr quality profile selection#5449herbalizer404 wants to merge 5 commits into
herbalizer404 wants to merge 5 commits into
Conversation
🦈 RepoShark Health Check
View full analysis → · Powered by RepoShark |
📝 WalkthroughWalkthroughAdds per-user Radarr and Sonarr quality-profile allowlists, persistence, role-based backend validation, selectable-profile endpoints, quality-only request dialogs, administration controls, 4K override handling, and automated tests. ChangesQuality profile permissions
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/Ombi.Store/Migrations/OmbiMySql/OmbiMySqlContextModelSnapshot.cs (1)
927-939: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSnapshot looks hand-edited rather than tool-generated.
UseMySqlIdentityColumnis a real PomeloPropertyBuilderextension, but calling it asMySqlPropertyBuilderExtensions.UseMySqlIdentityColumn(b.Property<int>("Id"))right after a separateb.Property<int>("Id")...HasColumnType("int")call is not howdotnet ef migrations addnormally emits this (elsewhere in this same file, auto-incrementIdproperties never get this explicit annotation call). Functionally harmless, but worth regenerating the snapshot via tooling to avoid future drift/spurious diffs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Ombi.Store/Migrations/OmbiMySql/OmbiMySqlContextModelSnapshot.cs` around lines 927 - 939, Regenerate the model snapshot using the project’s EF migration tooling instead of manually editing it. In the UserSelectableQualityProfile entity configuration, remove the explicit MySqlPropertyBuilderExtensions.UseMySqlIdentityColumn call if regeneration omits it, while preserving the generated property, key, index, and table configuration.src/Ombi.Store/Context/OmbiContext.cs (1)
56-63: 🗄️ Data Integrity & Integration | 🔵 TrivialAdd the
UserIdlength constraint to the Fluent API.
UserSelectableQualityProfile.UserIdis unbounded in the entity class andOmbiContext.cs, while the migrations/snapshots enforcemaxLength: 128. Configure it inOnModelCreatingso future migrations don’t report a model snapshot drift.🔧 Suggested fix
builder.Entity<UserSelectableQualityProfile>() .HasIndex(x => new { x.UserId, x.Application, x.QualityProfileId, x.Is4K }) .IsUnique(); builder.Entity<UserSelectableQualityProfile>() + .Property(x => x.UserId) + .HasMaxLength(128); + builder.Entity<UserSelectableQualityProfile>() .HasOne(x => x.User) .WithMany() .HasForeignKey(x => x.UserId) .OnDelete(DeleteBehavior.Cascade);[low_effort_and_high_yield]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Ombi.Store/Context/OmbiContext.cs` around lines 56 - 63, Update the UserSelectableQualityProfile mapping in OnModelCreating to configure UserId with the existing 128-character maximum length, alongside its index and foreign-key configuration. Keep the current uniqueness and cascade-delete behavior unchanged.src/Ombi/Controllers/V1/External/RadarrController.cs (1)
116-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate Admin/PowerUser bypass + allowlist-filter logic across
RadarrControllerandSonarrController. Both controllers independently re-implement "resolve current user → bypass filtering for Admin/PowerUser (two sequentialIsInRoleAsynccalls) → else queryUserSelectableQualityProfilefor allowedQualityProfileIds". Extracting this into a shared service would remove the duplication and reduce the risk of the two implementations silently diverging (e.g. one filtering byIs4K, the other not).
src/Ombi/Controllers/V1/External/RadarrController.cs#L116-L153: moveFilterSelectableProfilesinto a sharedIRepository<UserSelectableQualityProfile>-backed helper/service usable by both controllers, and combine the twoIsInRoleAsynccalls into a singleGetRolesAsynccheck.src/Ombi/Controllers/V1/External/SonarrController.cs#L86-L103: replace the inline bypass/allowlist logic inGetSelectableProfileswith a call to the same shared helper/service.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Ombi/Controllers/V1/External/RadarrController.cs` around lines 116 - 153, Extract RadarrController.FilterSelectableProfiles into a shared service/helper backed by IRepository<UserSelectableQualityProfile>, preserving the is4K-aware allowlist filtering and Admin/PowerUser bypass while replacing the two sequential IsInRoleAsync calls with a single GetRolesAsync check. In src/Ombi/Controllers/V1/External/RadarrController.cs#L116-153, inject and use the shared helper from both selectable-profile endpoints; in src/Ombi/Controllers/V1/External/SonarrController.cs#L86-103, replace the inline bypass/allowlist logic in GetSelectableProfiles with that same helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Ombi.Core/Engine/MovieRequestEngine.cs`:
- Around line 183-186: Update the existing-request update logic around
QualityPathOverride so SD and 4K requests do not overwrite a shared
QualityOverride value. Store or update the override in the variant-specific
field selected by the request’s 4K state, and ensure MovieSender.Send(model,
is4K) reads the matching variant value.
In
`@src/Ombi.Store/Migrations/OmbiMySql/20260723000000_UserSelectableQualityProfiles.cs`:
- Line 15: Replace the raw Sonarr role SQL in
src/Ombi.Store/Migrations/OmbiMySql/20260723000000_UserSelectableQualityProfiles.cs:15
with InsertRoleMySql(OmbiRoles.SelectSonarrQualityProfile). Replace the
corresponding raw SQL in
src/Ombi.Store/Migrations/OmbiPostgres/20260723000000_UserSelectableQualityProfiles.cs:16
with InsertRolePostgres(OmbiRoles.SelectSonarrQualityProfile), matching the
existing Radarr role migration pattern.
In
`@src/Ombi/ClientApp/src/app/shared/episode-request/episode-request.component.ts`:
- Around line 88-89: Move the assignment to this.data.series.requested out of
the pre-dialog flow and apply it only after AdminRequestDialogComponent resolves
with a successful submission. Preserve the cancellation path for non-admin
quality-only requests so closing the dialog leaves requested unchanged, and keep
the existing admin behavior intact.
In
`@src/Ombi/ClientApp/src/app/usermanagement/usermanagement-user.component.html`:
- Around line 91-110: Update the *ngIf conditions for selectableRadarrQualities,
selectableRadarr4KQualities, and selectableSonarrQualities to require a
non-empty array, such as checking each collection’s length, so fields are hidden
when profile lists resolve to [].
In `@src/Ombi/Controllers/V1/IdentityController.cs`:
- Around line 1089-1109: Wrap the delete-and-reinsert workflow in
ReplaceSelectableProfiles within a database transaction. Begin the transaction
before DeleteRange(existing), perform AddRange(profiles) within the same
transaction, commit only after both operations succeed, and roll back on failure
so the existing selectable profiles are preserved if insertion fails.
---
Nitpick comments:
In `@src/Ombi.Store/Context/OmbiContext.cs`:
- Around line 56-63: Update the UserSelectableQualityProfile mapping in
OnModelCreating to configure UserId with the existing 128-character maximum
length, alongside its index and foreign-key configuration. Keep the current
uniqueness and cascade-delete behavior unchanged.
In `@src/Ombi.Store/Migrations/OmbiMySql/OmbiMySqlContextModelSnapshot.cs`:
- Around line 927-939: Regenerate the model snapshot using the project’s EF
migration tooling instead of manually editing it. In the
UserSelectableQualityProfile entity configuration, remove the explicit
MySqlPropertyBuilderExtensions.UseMySqlIdentityColumn call if regeneration omits
it, while preserving the generated property, key, index, and table
configuration.
In `@src/Ombi/Controllers/V1/External/RadarrController.cs`:
- Around line 116-153: Extract RadarrController.FilterSelectableProfiles into a
shared service/helper backed by IRepository<UserSelectableQualityProfile>,
preserving the is4K-aware allowlist filtering and Admin/PowerUser bypass while
replacing the two sequential IsInRoleAsync calls with a single GetRolesAsync
check. In src/Ombi/Controllers/V1/External/RadarrController.cs#L116-153, inject
and use the shared helper from both selectable-profile endpoints; in
src/Ombi/Controllers/V1/External/SonarrController.cs#L86-103, replace the inline
bypass/allowlist logic in GetSelectableProfiles with that same helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0c1c4bbb-ca70-4323-9932-f3b7ec417958
📒 Files selected for processing (44)
src/Ombi.Core.Tests/Engine/MovieRequestEngineTests.cssrc/Ombi.Core.Tests/Engine/TvRequestEngineTests.cssrc/Ombi.Core.Tests/Engine/V2/MovieRequestEngineTests.cssrc/Ombi.Core/Engine/MovieRequestEngine.cssrc/Ombi.Core/Engine/TvRequestEngine.cssrc/Ombi.Core/Helpers/TvShowRequestBuilder.cssrc/Ombi.Core/Models/UI/UserViewModel.cssrc/Ombi.Helpers/OmbiRoles.cssrc/Ombi.Store/Context/OmbiContext.cssrc/Ombi.Store/Entities/UserSelectableQualityProfile.cssrc/Ombi.Store/Migrations/OmbiMySql/20260722000000_SelectRadarrQualityProfileRole.cssrc/Ombi.Store/Migrations/OmbiMySql/20260723000000_UserSelectableQualityProfiles.cssrc/Ombi.Store/Migrations/OmbiMySql/OmbiMySqlContextModelSnapshot.cssrc/Ombi.Store/Migrations/OmbiPostgres/20260722000000_SelectRadarrQualityProfileRole.cssrc/Ombi.Store/Migrations/OmbiPostgres/20260723000000_UserSelectableQualityProfiles.cssrc/Ombi.Store/Migrations/OmbiPostgres/OmbiPostgresContextModelSnapshot.cssrc/Ombi.Store/Migrations/OmbiSqlite/20260722000000_SelectRadarrQualityProfileRole.cssrc/Ombi.Store/Migrations/OmbiSqlite/20260723000000_UserSelectableQualityProfiles.cssrc/Ombi.Store/Migrations/OmbiSqlite/OmbiSqliteContextModelSnapshot.cssrc/Ombi.Tests/Controllers/V1/External/RadarrControllerTests.cssrc/Ombi.Tests/Controllers/V1/External/SonarrControllerTests.cssrc/Ombi.Tests/Migrations/SelectRadarrQualityProfileRoleTests.cssrc/Ombi/ClientApp/src/app/discover/components/card/discover-card.component.spec.tssrc/Ombi/ClientApp/src/app/discover/components/card/discover-card.component.tssrc/Ombi/ClientApp/src/app/interfaces/IUser.tssrc/Ombi/ClientApp/src/app/media-details/components/movie/movie-details.component.spec.tssrc/Ombi/ClientApp/src/app/media-details/components/movie/movie-details.component.tssrc/Ombi/ClientApp/src/app/media-details/components/tv/panels/tv-request-grid/tv-request-grid.component.tssrc/Ombi/ClientApp/src/app/services/applications/radarr.service.spec.tssrc/Ombi/ClientApp/src/app/services/applications/radarr.service.tssrc/Ombi/ClientApp/src/app/services/applications/sonarr.service.spec.tssrc/Ombi/ClientApp/src/app/services/applications/sonarr.service.tssrc/Ombi/ClientApp/src/app/shared/admin-request-dialog/admin-request-dialog.component.htmlsrc/Ombi/ClientApp/src/app/shared/admin-request-dialog/admin-request-dialog.component.spec.tssrc/Ombi/ClientApp/src/app/shared/admin-request-dialog/admin-request-dialog.component.tssrc/Ombi/ClientApp/src/app/shared/episode-request/episode-request.component.spec.tssrc/Ombi/ClientApp/src/app/shared/episode-request/episode-request.component.tssrc/Ombi/ClientApp/src/app/usermanagement/usermanagement-user.component.htmlsrc/Ombi/ClientApp/src/app/usermanagement/usermanagement-user.component.tssrc/Ombi/Controllers/V1/External/RadarrController.cssrc/Ombi/Controllers/V1/External/SonarrController.cssrc/Ombi/Controllers/V1/IdentityController.cssrc/Ombi/wwwroot/translations/en.jsonsrc/Ombi/wwwroot/translations/fr.json
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Ombi.Core.Tests/Engine/MovieRequestEngineTests.cs (1)
213-216: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire the base movie-request permission in successful selector-role tests.
The positive cases pass only
SelectRadarrQualityProfile, so this helper returnsfalsefor the underlying movie-request role while the tests still expect a successful request. Require both roles in successful cases and add a selector-only rejection case; otherwise this suite permits the dedicated selection role to become a request-creation permission.Proposed test adjustment
- SetupMovieRequestRoles(OmbiRoles.SelectRadarrQualityProfile); + SetupMovieRequestRoles( + OmbiRoles.RequestMovie, + OmbiRoles.SelectRadarrQualityProfile);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Ombi.Core.Tests/Engine/MovieRequestEngineTests.cs` around lines 213 - 216, Update SetupMovieRequestRoles and the related successful selector-role tests so successful requests require both the base movie-request permission and SelectRadarrQualityProfile. Add a selector-only rejection test that verifies request creation is denied without the base permission, preserving the existing role-checking behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/Ombi.Store/Migrations/OmbiMySql/20260723000001_QualityOverride4K.Designer.cs`:
- Line 21: Regenerate the migration designer files using the repository’s .NET
SDK and EF Core/provider 8.0.x toolchain so their ProductVersion and provider
annotations match the configured versions. Apply this to
src/Ombi.Store/Migrations/OmbiMySql/20260723000001_QualityOverride4K.Designer.cs,
src/Ombi.Store/Migrations/OmbiPostgres/20260723000001_QualityOverride4K.Designer.cs,
and
src/Ombi.Store/Migrations/OmbiSqlite/20260723000001_QualityOverride4K.Designer.cs;
no manual version edits or unrelated migration changes are needed.
In
`@src/Ombi/ClientApp/src/app/media-details/components/movie/movie-details.component.ts`:
- Around line 338-352: After the successful update calls in the options flow,
synchronise this.movieRequest.qualityOverride and qualityOverride4K with the
selected profile values, including both values for RequestCombination.Both and
the applicable override for single-quality requests, before
setAdvancedOptions(result).
In
`@src/Ombi/ClientApp/src/app/media-details/components/movie/panels/movie-advanced-options/movie-advanced-options.component.html`:
- Around line 10-13: Update the mat-option elements in the movie
advanced-options template, including the 4K profile selector and the selectors
binding profileId and rootFolderId, to use numeric property binding for
profile.id or rootFolder.id instead of interpolated value attributes. Preserve
the existing option iteration and displayed names while ensuring all bound IDs
remain numbers.
---
Outside diff comments:
In `@src/Ombi.Core.Tests/Engine/MovieRequestEngineTests.cs`:
- Around line 213-216: Update SetupMovieRequestRoles and the related successful
selector-role tests so successful requests require both the base movie-request
permission and SelectRadarrQualityProfile. Add a selector-only rejection test
that verifies request creation is denied without the base permission, preserving
the existing role-checking behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f65be1f8-6345-4bb4-b557-6d93f3cee2b6
📒 Files selected for processing (35)
src/Ombi.Core.Tests/Engine/MovieRequestEngineTests.cssrc/Ombi.Core.Tests/Senders/MovieSenderTests.cssrc/Ombi.Core/Engine/MovieRequestEngine.cssrc/Ombi.Core/Models/Requests/MovieAdvancedOptions.cssrc/Ombi.Core/Senders/MovieSender.cssrc/Ombi.Store/Context/OmbiContext.cssrc/Ombi.Store/Entities/Requests/MovieRequests.cssrc/Ombi.Store/Migrations/OmbiMySql/20260723000000_UserSelectableQualityProfiles.cssrc/Ombi.Store/Migrations/OmbiMySql/20260723000001_QualityOverride4K.Designer.cssrc/Ombi.Store/Migrations/OmbiMySql/20260723000001_QualityOverride4K.cssrc/Ombi.Store/Migrations/OmbiMySql/OmbiMySqlContextModelSnapshot.cssrc/Ombi.Store/Migrations/OmbiPostgres/20260723000000_UserSelectableQualityProfiles.cssrc/Ombi.Store/Migrations/OmbiPostgres/20260723000001_QualityOverride4K.Designer.cssrc/Ombi.Store/Migrations/OmbiPostgres/20260723000001_QualityOverride4K.cssrc/Ombi.Store/Migrations/OmbiPostgres/OmbiPostgresContextModelSnapshot.cssrc/Ombi.Store/Migrations/OmbiSqlite/20260723000000_UserSelectableQualityProfiles.cssrc/Ombi.Store/Migrations/OmbiSqlite/20260723000001_QualityOverride4K.Designer.cssrc/Ombi.Store/Migrations/OmbiSqlite/20260723000001_QualityOverride4K.cssrc/Ombi.Store/Migrations/OmbiSqlite/OmbiSqliteContextModelSnapshot.cssrc/Ombi.Store/Repository/BaseRepository.cssrc/Ombi.Store/Repository/IExternalRepository.cssrc/Ombi.Store/Repository/IRepository.cssrc/Ombi.Tests/Migrations/SelectRadarrQualityProfileRoleTests.cssrc/Ombi/ClientApp/src/app/interfaces/IRadarr.tssrc/Ombi/ClientApp/src/app/interfaces/IRequestModel.tssrc/Ombi/ClientApp/src/app/media-details/components/movie/movie-details.component.spec.tssrc/Ombi/ClientApp/src/app/media-details/components/movie/movie-details.component.tssrc/Ombi/ClientApp/src/app/media-details/components/movie/panels/movie-advanced-options/movie-advanced-options.component.htmlsrc/Ombi/ClientApp/src/app/media-details/components/movie/panels/movie-advanced-options/movie-advanced-options.component.spec.tssrc/Ombi/ClientApp/src/app/media-details/components/movie/panels/movie-advanced-options/movie-advanced-options.component.tssrc/Ombi/ClientApp/src/app/shared/episode-request/episode-request.component.spec.tssrc/Ombi/ClientApp/src/app/shared/episode-request/episode-request.component.tssrc/Ombi/ClientApp/src/app/usermanagement/usermanagement-user.component.htmlsrc/Ombi/ClientApp/src/app/usermanagement/usermanagement-user.component.spec.tssrc/Ombi/Controllers/V1/IdentityController.cs
🚧 Files skipped from review as they are similar to previous changes (9)
- src/Ombi.Store/Migrations/OmbiPostgres/20260723000000_UserSelectableQualityProfiles.cs
- src/Ombi.Store/Context/OmbiContext.cs
- src/Ombi.Store/Migrations/OmbiMySql/20260723000000_UserSelectableQualityProfiles.cs
- src/Ombi.Store/Migrations/OmbiPostgres/OmbiPostgresContextModelSnapshot.cs
- src/Ombi.Store/Migrations/OmbiSqlite/20260723000000_UserSelectableQualityProfiles.cs
- src/Ombi.Store/Migrations/OmbiMySql/OmbiMySqlContextModelSnapshot.cs
- src/Ombi/ClientApp/src/app/usermanagement/usermanagement-user.component.html
- src/Ombi/Controllers/V1/IdentityController.cs
- src/Ombi.Core/Engine/MovieRequestEngine.cs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Ombi.Store/Migrations/OmbiPostgres/OmbiPostgresContextModelSnapshot.cs`:
- Line 172: Review the DateTime column mappings in
OmbiPostgresContextModelSnapshot before merging: preserve timestamp with time
zone for UTC instant values, or add an explicit tested migration conversion if
timestamp without time zone is required. Update the affected model snapshot and
corresponding migration consistently, ensuring existing persisted UTC data
retains its intended semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0a9c3129-570b-4859-aedf-d8f725d0eef3
📒 Files selected for processing (12)
src/Ombi.Core.Tests/Engine/MovieRequestEngineTests.cssrc/Ombi.Store/Migrations/OmbiMySql/20260723000001_QualityOverride4K.Designer.cssrc/Ombi.Store/Migrations/OmbiMySql/OmbiMySqlContextModelSnapshot.cssrc/Ombi.Store/Migrations/OmbiPostgres/20260723000001_QualityOverride4K.Designer.cssrc/Ombi.Store/Migrations/OmbiPostgres/OmbiPostgresContextModelSnapshot.cssrc/Ombi.Store/Migrations/OmbiSqlite/20260723000001_QualityOverride4K.Designer.cssrc/Ombi.Store/Migrations/OmbiSqlite/OmbiSqliteContextModelSnapshot.cssrc/Ombi/ClientApp/src/app/media-details/components/movie/movie-details.component.spec.tssrc/Ombi/ClientApp/src/app/media-details/components/movie/movie-details.component.tssrc/Ombi/ClientApp/src/app/media-details/components/movie/panels/movie-advanced-options/movie-advanced-options.component.htmlsrc/Ombi/ClientApp/src/app/media-details/components/movie/panels/movie-advanced-options/movie-advanced-options.component.spec.tssrc/Ombi/ClientApp/src/app/media-details/components/movie/panels/movie-advanced-options/movie-advanced-options.component.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- src/Ombi/ClientApp/src/app/media-details/components/movie/panels/movie-advanced-options/movie-advanced-options.component.html
- src/Ombi/ClientApp/src/app/media-details/components/movie/panels/movie-advanced-options/movie-advanced-options.component.spec.ts
- src/Ombi/ClientApp/src/app/media-details/components/movie/movie-details.component.spec.ts
- src/Ombi/ClientApp/src/app/media-details/components/movie/movie-details.component.ts
- src/Ombi.Store/Migrations/OmbiPostgres/20260723000001_QualityOverride4K.Designer.cs
- src/Ombi.Store/Migrations/OmbiMySql/20260723000001_QualityOverride4K.Designer.cs
- src/Ombi.Store/Migrations/OmbiSqlite/20260723000001_QualityOverride4K.Designer.cs
- src/Ombi/ClientApp/src/app/media-details/components/movie/panels/movie-advanced-options/movie-advanced-options.component.ts
- src/Ombi.Store/Migrations/OmbiSqlite/OmbiSqliteContextModelSnapshot.cs
- src/Ombi.Core.Tests/Engine/MovieRequestEngineTests.cs
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Ombi.Store/Migrations/OmbiPostgres/OmbiPostgresContextModelSnapshot.cs (1)
1032-1034: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winMake
UserSelectableQualityProfile.UserIdrequired.The snapshot currently models this foreign key as nullable, allowing orphaned allowlist rows with
NULLuser IDs; PostgreSQL can also permit multiple such rows despite the composite unique index. Mark the property and relationship as required in the entity configuration, then regenerate the migration and snapshot.Also applies to: 1331-1334
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Ombi.Store/Migrations/OmbiPostgres/OmbiPostgresContextModelSnapshot.cs` around lines 1032 - 1034, Update the UserSelectableQualityProfile entity configuration in OmbiPostgresContextModelSnapshot so UserId is non-nullable and its foreign-key relationship is required, preventing orphaned allowlist rows. Regenerate the corresponding EF migration and model snapshot so the database column and relationship constraints match the required UserId contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/Ombi.Store/Migrations/OmbiPostgres/OmbiPostgresContextModelSnapshot.cs`:
- Around line 1032-1034: Update the UserSelectableQualityProfile entity
configuration in OmbiPostgresContextModelSnapshot so UserId is non-nullable and
its foreign-key relationship is required, preventing orphaned allowlist rows.
Regenerate the corresponding EF migration and model snapshot so the database
column and relationship constraints match the required UserId contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7358c001-7405-400d-9401-3b37575e0019
📒 Files selected for processing (2)
src/Ombi.Store/Migrations/OmbiPostgres/20260723000001_QualityOverride4K.Designer.cssrc/Ombi.Store/Migrations/OmbiPostgres/OmbiPostgresContextModelSnapshot.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Ombi.Store/Migrations/OmbiPostgres/20260723000001_QualityOverride4K.Designer.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This adds dedicated
SelectRadarrQualityProfileandSelectSonarrQualityProfileroles so trusted users can choose a quality profile per movie or TV request without receiving Power User access. Administrators can explicitly restrict each user to an allowlist of profiles.What changed
SelectRadarrQualityProfileandSelectSonarrQualityProfileroles.UserSelectableQualityProfilerelation with a unique per-user/application/profile/instance constraint and cascade deletion.idandname.Authorization and fail-closed behavior
For normal users, selecting a profile requires both the corresponding request permission and the dedicated selection role:
RequestMovie+SelectRadarrQualityProfile;RequestTv+SelectSonarrQualityProfile.A normal user with the dedicated role but no allowlisted profiles sees no selectable profiles. New profiles added later in Radarr or Sonarr are not automatically exposed.
Admin and Power User behavior remains unrestricted.
Server-side enforcement
The backend independently verifies:
The dedicated roles do not grant access to API keys, connection settings, root-folder selection, language-profile selection, request-on-behalf controls, user administration, or other Power User capabilities.
Test plan
Ombi.Core.Testssuite: exit 0Ombi.Testssuite: exit 0git diff --checkExisting dependency advisory and SCSS budget warnings are unchanged by this PR.
Database coverage
Branch
developatadd609c82e699cddbb627f078e197925c93f3e32feature/per-user-arr-quality-profiles0a4f82fe2Submitted from the contributor fork; no changes were pushed directly to the upstream repository.
Summary by CodeRabbit