feat: add BTCPay min/max compatibility management for plugin versions#174
Merged
feat: add BTCPay min/max compatibility management for plugin versions#174
Conversation
|
Note
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 2.70% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title accurately summarizes the main change: adding BTCPay min/max compatibility management for plugin versions, which is the primary feature across the entire changeset. |
| Description check | ✅ Passed | The description is clearly related to the changeset, explaining the introduction of BTCPay max range support, persistence of compatibility values, application to public endpoints, and UI support for editing from multiple locations. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feat/btcpay-compatibility
📝 Coding Plan
- Generate coding plan for human review comments
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 @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
PluginBuilder/Controllers/AdminController.cs (1)
177-195:⚠️ Potential issue | 🟠 MajorMirror the owner-settings URL validation here.
The admin form now edits
GitRepositoryandDocumentation, but this POST only validatesVideoUrl. Invalid or non-HTTPS repository/docs URLs will be persisted here and then reused as defaults on future builds.🛠️ Possible fix
+ if (string.IsNullOrEmpty(model.PluginSettings.GitRepository) || + !Uri.TryCreate(model.PluginSettings.GitRepository, UriKind.Absolute, out var gitRepoUri) || + gitRepoUri.Scheme != Uri.UriSchemeHttps) + { + ModelState.AddModelError($"{nameof(PluginEditViewModel.PluginSettings)}.{nameof(PluginSettings.GitRepository)}", + "Git repository is required and must be an HTTPS URL."); + await PopulatePluginEditViewModel(conn, pluginSlug, model); + return View(model); + } + + if (!string.IsNullOrEmpty(model.PluginSettings.Documentation) && + (!Uri.TryCreate(model.PluginSettings.Documentation, UriKind.Absolute, out var docUri) || + docUri.Scheme != Uri.UriSchemeHttps)) + { + ModelState.AddModelError($"{nameof(PluginEditViewModel.PluginSettings)}.{nameof(PluginSettings.Documentation)}", + "Documentation must be an HTTPS URL."); + await PopulatePluginEditViewModel(conn, pluginSlug, model); + return View(model); + } + if (!string.IsNullOrEmpty(model.PluginSettings.VideoUrl)) { if (!Uri.TryCreate(model.PluginSettings.VideoUrl, UriKind.Absolute, out var videoUri) || videoUri.Scheme != Uri.UriSchemeHttps) {Also applies to: 213-220
🧹 Nitpick comments (4)
PluginBuilder.Tests/PluginTests/ImportReviewUITests.cs (1)
65-65: Use a stricter locator for the Reviews tab.
a:has-text('Reviews')is substring-based and will become ambiguous as soon as another Reviews link appears on the page. A role locator or test id would make this flow much less brittle.Also applies to: 87-87
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PluginBuilder.Tests/PluginTests/ImportReviewUITests.cs` at line 65, The click uses a brittle substring locator ("a:has-text('Reviews')") in ImportReviewUITests (the await t.Page.ClickAsync call) which can become ambiguous; replace it with a stricter locator such as a role or test-id locator (e.g. use Page.GetByRole(AriaRole.Tab, new { name = "Reviews" }) or Page.Locator("[data-testid=\"reviews-tab\"]") depending on your markup) for both occurrences (the ClickAsync calls at the two locations) so the test targets the specific Reviews tab reliably.PluginBuilder/APIModels/PublishedVersion.cs (1)
12-13: Explicitly pin the JSON property names for the new BTCPay fields.The Swagger specification confirms the API serializes these properties to
btcpayMinVersionandbtcpayMaxVersion, relying on ASP.NET Core's default camelCase naming convention. While this currently works, making the mapping explicit improves maintainability and reduces implicit coupling to the serializer's naming strategy.Suggested fix
+ [JsonProperty("btcpayMinVersion")] public string BTCPayMinVersion { get; set; } + [JsonProperty("btcpayMaxVersion")] public string BTCPayMaxVersion { get; set; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PluginBuilder/APIModels/PublishedVersion.cs` around lines 12 - 13, Add explicit JSON property name attributes for the two new properties on the PublishedVersion class: annotate BTCPayMinVersion with JsonPropertyName("btcpayMinVersion") and BTCPayMaxVersion with JsonPropertyName("btcpayMaxVersion") (from System.Text.Json.Serialization) so the serialized names no longer rely on the global camelCase policy; ensure the using for System.Text.Json.Serialization is present or add it.PluginBuilder/PluginVersion.cs (1)
5-5: Consider implementingIEquatable<PluginVersion>for consistency.When implementing
IComparable<T>, it's typically recommended to also implementIEquatable<T>and overrideEquals/GetHashCodeto maintain consistency. This prevents situations wherea.CompareTo(b) == 0buta.Equals(b)returnsfalse.This is optional since the current usage may not require equality semantics, but worth considering for future-proofing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PluginBuilder/PluginVersion.cs` at line 5, Add IEquatable<PluginVersion> to the PluginVersion declaration and implement Equals(PluginVersion) so it returns true exactly when CompareTo(other) == 0; also override Equals(object) to call the typed Equals and override GetHashCode to produce a hash consistent with the equality comparison (use the same fields used by CompareTo). Optionally add operator == and != forwarding to Equals. Update the class signature PluginVersion, the CompareTo method usage, and ensure all equality logic references the same member fields so CompareTo(a,b)==0 implies Equals(a,b)==true.PluginBuilder/Data/Scripts/19.BTCPayCompatibility.sql (1)
30-50: Functionget_all_versionsduplicates logic fromget_latest_versions.Both functions share nearly identical filtering logic (lines 36-45 vs 15-23). Consider extracting the common WHERE clause into a reusable SQL fragment or view to avoid duplication.
This is a minor refactor suggestion - the current implementation is correct and functional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PluginBuilder/Data/Scripts/19.BTCPayCompatibility.sql` around lines 30 - 50, Extract the duplicated filtering logic used in get_all_versions and get_latest_versions into a single reusable database object (either a view named version_candidates or a SQL function named filter_versions(btcpayVersion INT[], includePreRelease BOOLEAN) that returns SETOF versions) and then change both functions to SELECT from that view/function instead of repeating the WHERE block; update get_all_versions (and get_latest_versions) to use the new version_candidates view or call filter_versions(...) in place of the current WHERE clause and preserve the existing ordering/CTE logic (latest_versions) and output columns.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@PluginBuilder.Tests/AdminTests/AdminBTCPayCompatibilityUITests.cs`:
- Around line 92-104: The test never creates a prerelease version row; after
parsing the manifest and calling conn.SetPluginSettings, call the method that
records a build as a prerelease so assertions exercise the prerelease path.
Specifically, invoke SetVersionBuild (or the DB helper with that name) with the
plugin slug, the manifest version (e.g., manifest.Version), the build id
(fullBuildId.BuildId) and preRelease: true to create the prerelease version row
before running the rest of the test.
In `@PluginBuilder/Controllers/AdminController.cs`:
- Around line 663-672: The admin view is building
PublishedPluginVersionAdminViewModel from stored row fields and ignores the
resolved effective compatibility range in ManifestInfo, causing blank/incorrect
min/max shown and risking overwriting manifest bounds; update the projection
that returns PublishedPluginVersionAdminViewModel (the lambda producing
Version/BtcPayMinVersion/BtcPayMaxVersion/HasBtcPayMinVersionOverride/HasBtcPayMaxVersionOverride
and ManifestCondition) to call the same effective-range resolver used by
PluginController.Build (use ManifestInfo to compute effective min/max and
override flags) and map those resolved values through FormatVersion and
TryGetBtcPayDependencyCondition so the Versions tab displays and preserves the
effective compatibility range rather than raw stored arrays.
- Around line 610-615: PopulatePluginEditViewModel currently overwrites the
user's posted Visibility with the persisted value via model.Visibility =
plugin.Visibility; change this so the posted value is preserved on validation
failure by only assigning the persisted value when the model's Visibility is not
already set (e.g., use null-coalescing assignment: model.Visibility ??=
plugin.Visibility or equivalent check). Update the assignment in
PopulatePluginEditViewModel (referencing model.Visibility and plugin.Visibility)
to avoid clobbering the submitted value.
In `@PluginBuilder/Controllers/ApiController.cs`:
- Around line 529-554: CreatePublishedVersion is not setting the
PublishedVersion.VideoUrl property, so VideoUrl is always null; update
CreatePublishedVersion to populate VideoUrl (e.g., VideoUrl = settings?.VideoUrl
?? manifestInfo["VideoUrl"]?.ToString() or the correct manifest key) so the
PublishedVersion returned by the controller includes the video link as defined
in PluginBuilder/APIModels/PublishedVersion.cs; locate the
CreatePublishedVersion method and add the VideoUrl assignment to the returned
PublishedVersion initialiser.
In `@PluginBuilder/Controllers/PluginController.cs`:
- Around line 514-516: The GET-side rendering uses
manifest?.BTCPayMinVersion/null when stored parts are missing which differs from
the POST defaults and causes blank/incorrect UI; change the logic that sets
vm.BTCPayMinVersion and vm.BTCPayMaxVersion to mirror the POST defaults by using
row.btcpay_min_ver/row.btcpay_max_ver when present, otherwise fall back to
manifest?.BTCPayMinVersion ?? PluginVersion.Zero for the min and to
manifest?.BTCPayMaxVersion (not null) for the max so the effective BTCPay range
matches the POST path and avoids unintended maxOverrideEnabled flips.
In `@PluginBuilder/PluginManifest.cs`:
- Around line 43-55: The TryParse method currently only catches FormatException
but Parse(string, bool) can also throw JsonException or
InvalidOperationException; update TryParse to catch those exceptions as well
(catch JsonException and InvalidOperationException in addition to
FormatException) and on any of them set manifest = null! and return false so
malformed JSON or null-deserialization from Parse doesn't propagate; keep the
existing behavior of returning true only when Parse succeeds.
In `@PluginBuilder/Views/Plugin/Build.cshtml`:
- Around line 157-190: The BTCPay compatibility row is only shown when a Min or
Max exists, leaving no trigger to open the `#btcpayCompatibilityModal` for builds
with no manifest values; change the outer condition to render the row whenever
the build exists and either there is an existing compatibility value OR the
current user can edit it. Concretely, update the if condition that references
Model.BTCPayMinVersion and Model.BTCPayMaxVersion to: check Model.Version !=
null && (Model.CanEditBTCPayCompatibility ||
!string.IsNullOrEmpty(Model.BTCPayMinVersion) ||
!string.IsNullOrEmpty(Model.BTCPayMaxVersion)) so the compatibility-edit-trigger
button and modal target (`#btcpayCompatibilityModal`) are present even when both
min and max are empty.
In `@PluginBuilder/wwwroot/swagger/v1/swagger.json`:
- Around line 779-780: The swagger example for the version condition is
inconsistent with the validation in PluginManifest.cs; update the example string
value from ">=1.9.0 && <2.0.0" to use "<=" for the upper bound (i.e. ">=1.9.0 &&
<=2.0.0") so it matches the accepted format enforced by the regex and the error
message that expects ">= min" or ">= min && <= max".
- Around line 546-556: Swagger uses camelCase names but the API serializes
PublishedVersion properties with PascalCase; update the PublishedVersion class
to decorate the properties BTCPayMinVersion and BTCPayMaxVersion with
Newtonsoft.Json attributes (e.g., [JsonProperty("btcpayMinVersion")] and
[JsonProperty("btcpayMaxVersion")]) so the output matches swagger, or
alternatively change the global JSON settings in Program.cs where
AddNewtonsoftJson() is configured to use a camelCase naming strategy; pick one
approach and apply it to ensure the serialized names match the swagger schema.
---
Nitpick comments:
In `@PluginBuilder.Tests/PluginTests/ImportReviewUITests.cs`:
- Line 65: The click uses a brittle substring locator ("a:has-text('Reviews')")
in ImportReviewUITests (the await t.Page.ClickAsync call) which can become
ambiguous; replace it with a stricter locator such as a role or test-id locator
(e.g. use Page.GetByRole(AriaRole.Tab, new { name = "Reviews" }) or
Page.Locator("[data-testid=\"reviews-tab\"]") depending on your markup) for both
occurrences (the ClickAsync calls at the two locations) so the test targets the
specific Reviews tab reliably.
In `@PluginBuilder/APIModels/PublishedVersion.cs`:
- Around line 12-13: Add explicit JSON property name attributes for the two new
properties on the PublishedVersion class: annotate BTCPayMinVersion with
JsonPropertyName("btcpayMinVersion") and BTCPayMaxVersion with
JsonPropertyName("btcpayMaxVersion") (from System.Text.Json.Serialization) so
the serialized names no longer rely on the global camelCase policy; ensure the
using for System.Text.Json.Serialization is present or add it.
In `@PluginBuilder/Data/Scripts/19.BTCPayCompatibility.sql`:
- Around line 30-50: Extract the duplicated filtering logic used in
get_all_versions and get_latest_versions into a single reusable database object
(either a view named version_candidates or a SQL function named
filter_versions(btcpayVersion INT[], includePreRelease BOOLEAN) that returns
SETOF versions) and then change both functions to SELECT from that view/function
instead of repeating the WHERE block; update get_all_versions (and
get_latest_versions) to use the new version_candidates view or call
filter_versions(...) in place of the current WHERE clause and preserve the
existing ordering/CTE logic (latest_versions) and output columns.
In `@PluginBuilder/PluginVersion.cs`:
- Line 5: Add IEquatable<PluginVersion> to the PluginVersion declaration and
implement Equals(PluginVersion) so it returns true exactly when CompareTo(other)
== 0; also override Equals(object) to call the typed Equals and override
GetHashCode to produce a hash consistent with the equality comparison (use the
same fields used by CompareTo). Optionally add operator == and != forwarding to
Equals. Update the class signature PluginVersion, the CompareTo method usage,
and ensure all equality logic references the same member fields so
CompareTo(a,b)==0 implies Equals(a,b)==true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e60f7833-7c46-4a81-b219-417653f2f565
📒 Files selected for processing (28)
PluginBuilder.Tests/AdminTests/AdminBTCPayCompatibilityUITests.csPluginBuilder.Tests/BTCPayCompatibilityTests.csPluginBuilder.Tests/PluginTests/BuildUITests.csPluginBuilder.Tests/PluginTests/ImportReviewUITests.csPluginBuilder.Tests/PluginTests/PluginCleanupTests.csPluginBuilder.Tests/PluginTests/VideoUITests.csPluginBuilder.Tests/PublicTests/PublicDirectoryUITests.csPluginBuilder.Tests/UnitTest1.csPluginBuilder/APIModels/PublishedVersion.csPluginBuilder/Controllers/AdminController.csPluginBuilder/Controllers/ApiController.csPluginBuilder/Controllers/PluginController.csPluginBuilder/Data/Scripts/19.BTCPayCompatibility.sqlPluginBuilder/PluginManifest.csPluginBuilder/PluginVersion.csPluginBuilder/Services/BuildService.csPluginBuilder/Util/Extensions/NpgsqlConnectionExtensions.csPluginBuilder/ViewModels/Admin/AdminPluginViewModel.csPluginBuilder/ViewModels/Admin/PluginEditViewModel.csPluginBuilder/ViewModels/BuildViewModel.csPluginBuilder/ViewModels/PluginOwnersTableViewModel.csPluginBuilder/ViewModels/Shared/BTCPayCompatibilityModalViewModel.csPluginBuilder/Views/Admin/ListPlugins.cshtmlPluginBuilder/Views/Admin/PluginEdit.cshtmlPluginBuilder/Views/Plugin/Build.cshtmlPluginBuilder/Views/Shared/_BTCPayCompatibilityModal.cshtmlPluginBuilder/Views/Shared/_PluginOwnersTable.cshtmlPluginBuilder/wwwroot/swagger/v1/swagger.json
PluginBuilder.Tests/AdminTests/AdminBTCPayCompatibilityUITests.cs
Outdated
Show resolved
Hide resolved
thgO-O
force-pushed
the
feat/btcpay-compatibility
branch
from
5cfe563 to
2316ebf
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
PluginBuilder/Views/Plugin/Build.cshtml (1)
157-190:⚠️ Potential issue | 🟠 MajorKeep the compatibility control visible when no range is set.
When a build has no manifest BTCPay condition and no override yet, this block never renders, so the user loses the only visible trigger for
#btcpayCompatibilityModaland cannot add compatibility from the build page at all.♻️ Suggested fix
- `@if` (Model.Version != null && (!string.IsNullOrEmpty(Model.BTCPayMinVersion) || !string.IsNullOrEmpty(Model.BTCPayMaxVersion))) + `@if` (Model.Version != null && (Model.CanEditBTCPayCompatibility || + !string.IsNullOrEmpty(Model.BTCPayMinVersion) || + !string.IsNullOrEmpty(Model.BTCPayMaxVersion))) { <tr> <th class="fw-semibold">BTCPay compatibility</th> <td> <div class="d-flex flex-wrap align-items-center gap-3"> - <span>Min: <code>@Model.BTCPayMinVersion</code></span> + <span> + Min: + `@if` (!string.IsNullOrEmpty(Model.BTCPayMinVersion)) + { + <code>@Model.BTCPayMinVersion</code> + } + else + { + <span class="text-muted">None</span> + } + </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PluginBuilder/Views/Plugin/Build.cshtml` around lines 157 - 190, The BTCPay compatibility row is hidden when both BTCPayMinVersion and BTCPayMaxVersion are empty, removing the only trigger for `#btcpayCompatibilityModal`; update the render condition so the row also appears when Model.CanEditBTCPayCompatibility is true. Concretely, change the if around the BTCPay compatibility block (the check using Model.Version, Model.BTCPayMinVersion, and Model.BTCPayMaxVersion) to include Model.CanEditBTCPayCompatibility (e.g., render when Model.Version != null && (there is a min/max OR Model.CanEditBTCPayCompatibility)) so the edit button and modal trigger are always visible to users who can edit, and keep the existing inner display logic that shows "None" when values are empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@PluginBuilder/Controllers/AdminController.cs`:
- Around line 279-352: The manifest strict-parse result isn't reused so override
clearing can be driven by the non-strict fallback; ensure you only clear
overrides when manifest defaults were successfully parsed with
strictBTCPayVersionCondition. Change the logic around
PluginManifest.TryParse/PluginManifest.Parse to keep a strict-parse result (e.g.
strictManifest and a bool strictParsed) and use
strictManifest.BTCPayMinVersion/BTCPayMaxVersion (not the non-strict parse) when
computing clearMinOverride and clearMaxOverride; set clearMinOverride =
useManifestDefaults || (strictParsed && VersionEquals(candidateMinVersion,
strictManifestMin)) and similarly for clearMaxOverride using candidateMaxVersion
and strictManifestMax. Apply the same fix to the corresponding code in
PluginBuilder/Controllers/PluginController.cs (same variable names:
useManifestDefaults, manifest, candidateMinVersion, candidateMaxVersion,
clearMinOverride, clearMaxOverride).
In `@PluginBuilder/PluginManifest.cs`:
- Around line 10-12: The manifest version parts parsed by the
BTCPayVersionConditionRegex can be 1–4 segments but PostgreSQL INT[] compares
arrays lexicographically, so normalize the VersionParts to a fixed width (4)
before saving: in the code path that constructs/persists the VersionParts
property (where you parse using BTCPayVersionConditionRegex), pad with trailing
zeros to length 4 (e.g., [2,1] -> [2,1,0,0]) so DB comparisons behave
numerically; alternatively, if you prefer query-side fixes, change the
persistence/query logic that compares VersionParts to only compare the first N
elements (e.g., compare slices or individual elements) but the simplest fix is
to pad VersionParts to 4 ints prior to persistence.
In `@PluginBuilder/Views/Admin/PluginEdit.cshtml`:
- Around line 147-153: Update the view and controller to bind the validation to
the file input: change the validation span in PluginEdit.cshtml from
asp-validation-for="PluginSettings.Logo" to asp-validation-for="LogoFile" so
errors appear next to the upload control, and in
PluginBuilder/Controllers/AdminController.cs move any ModelState.AddModelError
keys/validation for upload failures to the "LogoFile" key (where you currently
add errors under the PluginSettings.Logo key) so both exception and
invalid-image validation paths surface in the same place; search for LogoFile,
PluginSettings.Logo and any ModelState.AddModelError calls in the controller
method that handles the logo upload/update to make the changes.
---
Duplicate comments:
In `@PluginBuilder/Views/Plugin/Build.cshtml`:
- Around line 157-190: The BTCPay compatibility row is hidden when both
BTCPayMinVersion and BTCPayMaxVersion are empty, removing the only trigger for
`#btcpayCompatibilityModal`; update the render condition so the row also appears
when Model.CanEditBTCPayCompatibility is true. Concretely, change the if around
the BTCPay compatibility block (the check using Model.Version,
Model.BTCPayMinVersion, and Model.BTCPayMaxVersion) to include
Model.CanEditBTCPayCompatibility (e.g., render when Model.Version != null &&
(there is a min/max OR Model.CanEditBTCPayCompatibility)) so the edit button and
modal trigger are always visible to users who can edit, and keep the existing
inner display logic that shows "None" when values are empty.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aa290b1d-51a6-4485-9386-2576257c5777
📒 Files selected for processing (28)
PluginBuilder.Tests/AdminTests/AdminBTCPayCompatibilityUITests.csPluginBuilder.Tests/BTCPayCompatibilityTests.csPluginBuilder.Tests/PluginTests/BuildUITests.csPluginBuilder.Tests/PluginTests/ImportReviewUITests.csPluginBuilder.Tests/PluginTests/PluginCleanupTests.csPluginBuilder.Tests/PluginTests/VideoUITests.csPluginBuilder.Tests/PublicTests/PublicDirectoryUITests.csPluginBuilder.Tests/UnitTest1.csPluginBuilder/APIModels/PublishedVersion.csPluginBuilder/Controllers/AdminController.csPluginBuilder/Controllers/ApiController.csPluginBuilder/Controllers/PluginController.csPluginBuilder/Data/Scripts/19.BTCPayCompatibility.sqlPluginBuilder/PluginManifest.csPluginBuilder/PluginVersion.csPluginBuilder/Services/BuildService.csPluginBuilder/Util/Extensions/NpgsqlConnectionExtensions.csPluginBuilder/ViewModels/Admin/AdminPluginViewModel.csPluginBuilder/ViewModels/Admin/PluginEditViewModel.csPluginBuilder/ViewModels/BuildViewModel.csPluginBuilder/ViewModels/PluginOwnersTableViewModel.csPluginBuilder/ViewModels/Shared/BTCPayCompatibilityModalViewModel.csPluginBuilder/Views/Admin/ListPlugins.cshtmlPluginBuilder/Views/Admin/PluginEdit.cshtmlPluginBuilder/Views/Plugin/Build.cshtmlPluginBuilder/Views/Shared/_BTCPayCompatibilityModal.cshtmlPluginBuilder/Views/Shared/_PluginOwnersTable.cshtmlPluginBuilder/wwwroot/swagger/v1/swagger.json
✅ Files skipped from review due to trivial changes (9)
- PluginBuilder.Tests/PluginTests/PluginCleanupTests.cs
- PluginBuilder.Tests/PluginTests/ImportReviewUITests.cs
- PluginBuilder/ViewModels/PluginOwnersTableViewModel.cs
- PluginBuilder/APIModels/PublishedVersion.cs
- PluginBuilder/ViewModels/BuildViewModel.cs
- PluginBuilder/Views/Admin/ListPlugins.cshtml
- PluginBuilder/Views/Shared/_BTCPayCompatibilityModal.cshtml
- PluginBuilder/ViewModels/Shared/BTCPayCompatibilityModalViewModel.cs
- PluginBuilder/Views/Shared/_PluginOwnersTable.cshtml
🚧 Files skipped from review as they are similar to previous changes (6)
- PluginBuilder.Tests/UnitTest1.cs
- PluginBuilder/PluginVersion.cs
- PluginBuilder/Services/BuildService.cs
- PluginBuilder.Tests/PublicTests/PublicDirectoryUITests.cs
- PluginBuilder/wwwroot/swagger/v1/swagger.json
- PluginBuilder/Util/Extensions/NpgsqlConnectionExtensions.cs
thgO-O
force-pushed
the
feat/btcpay-compatibility
branch
from
2316ebf to
bb3115f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
PluginBuilder/Views/Plugin/Build.cshtml (1)
157-190:⚠️ Potential issue | 🟡 MinorThe compatibility row is hidden when no values exist, blocking access to the edit modal.
The condition on line 157 requires
BTCPayMinVersionorBTCPayMaxVersionto be non-empty. However, if a build has no manifest BTCPay dependency condition and no existing override, both values will be empty, hiding the entire row including the edit trigger button. The modal is rendered (lines 120-123), but there's no way to open it.🔧 Suggested fix
- `@if` (Model.Version != null && (!string.IsNullOrEmpty(Model.BTCPayMinVersion) || !string.IsNullOrEmpty(Model.BTCPayMaxVersion))) + `@if` (Model.Version != null && (Model.CanEditBTCPayCompatibility || !string.IsNullOrEmpty(Model.BTCPayMinVersion) || !string.IsNullOrEmpty(Model.BTCPayMaxVersion))) { <tr> <th class="fw-semibold">BTCPay compatibility</th> <td> <div class="d-flex flex-wrap align-items-center gap-3"> - <span>Min: <code>@Model.BTCPayMinVersion</code></span> + <span> + Min: + `@if` (!string.IsNullOrEmpty(Model.BTCPayMinVersion)) + { + <code>@Model.BTCPayMinVersion</code> + } + else + { + <span class="text-muted">None</span> + } + </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PluginBuilder/Views/Plugin/Build.cshtml` around lines 157 - 190, The BTCPay compatibility row is currently only rendered when Model.Version is non-null and one of Model.BTCPayMinVersion or Model.BTCPayMaxVersion is non-empty, which hides the row (and the edit button) when both are empty; change the rendering condition to also show the row when Model.CanEditBTCPayCompatibility is true so the edit trigger can be accessed even if both Model.BTCPayMinVersion and Model.BTCPayMaxVersion are empty—update the if condition around the compatibility row in Build.cshtml to include Model.CanEditBTCPayCompatibility (keeping the existing checks for Model.Version and the modal target `#btcpayCompatibilityModal` and the compatibility-edit-trigger button intact).
🧹 Nitpick comments (1)
PluginBuilder.Tests/PluginTests/BuildUITests.cs (1)
131-132: Lookup the BTCPay dependency explicitly before mutating it.Line 132 assumes
Dependencies[0]is theBTCPayServerentry. If the manifest ever gains another dependency first, this test stops exercising the unsupported-BTCPay-condition path and starts failing for the wrong reason.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PluginBuilder.Tests/PluginTests/BuildUITests.cs` around lines 131 - 132, The test currently mutates Dependencies[0] assuming it's BTCPayServer; instead locate the BTCPayServer dependency by searching unsupportedManifest["Dependencies"] for the entry whose "Name" (or property identifying it) equals "BTCPayServer", then set that entry's "Condition" to ">= 1.0.0 && < 2.0.0"; update the code around the unsupportedManifest variable (reading ManifestInfoJson) to find the correct dependency token (e.g., via LINQ/JToken.Children().First/Single with a predicate) and modify its "Condition" rather than indexing [0].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@PluginBuilder/Controllers/PluginController.cs`:
- Around line 537-545: The query in PluginController.cs selects v.btcpay_min_ver
and v.btcpay_max_ver (mapped as int[] btcpay_min_ver, int[]? btcpay_max_ver)
causing Dapper to fail on NULL legacy rows even though this endpoint only needs
manifest_info; update the SQL passed to conn.QueryFirstOrDefaultAsync (and the
mapped tuple type used for row) to select only b.manifest_info so that the
btcpay_* columns are not requested or mapped, leaving the rest of the method
(row usage/validation of manifest_info) unchanged.
In `@PluginBuilder/Views/Admin/PluginEdit.cshtml`:
- Around line 313-356: The hidden sections (`#existingUserSection`,
`#manualIdentitySection`, `#wwwExtras`) still post their inputs (e.g.,
SelectedUserId, ReviewerName, WwwDisplayName, WwwAvatarUrl,
Source/platformSelect) causing ambiguous identity on submit; update the form
submit handler (triggered by the form submit or by the element bound to
LinkExistingUser) to disable (set disabled=true) or remove the name attribute
for all inputs/selects inside the section(s) that are hidden so only the visible
section's controls are submitted—use the LinkExistingUser checkbox state and the
IDs existingUserSection, manualIdentitySection and wwwExtras to decide which
inputs to disable/enable before allowing the submit.
---
Duplicate comments:
In `@PluginBuilder/Views/Plugin/Build.cshtml`:
- Around line 157-190: The BTCPay compatibility row is currently only rendered
when Model.Version is non-null and one of Model.BTCPayMinVersion or
Model.BTCPayMaxVersion is non-empty, which hides the row (and the edit button)
when both are empty; change the rendering condition to also show the row when
Model.CanEditBTCPayCompatibility is true so the edit trigger can be accessed
even if both Model.BTCPayMinVersion and Model.BTCPayMaxVersion are empty—update
the if condition around the compatibility row in Build.cshtml to include
Model.CanEditBTCPayCompatibility (keeping the existing checks for Model.Version
and the modal target `#btcpayCompatibilityModal` and the
compatibility-edit-trigger button intact).
---
Nitpick comments:
In `@PluginBuilder.Tests/PluginTests/BuildUITests.cs`:
- Around line 131-132: The test currently mutates Dependencies[0] assuming it's
BTCPayServer; instead locate the BTCPayServer dependency by searching
unsupportedManifest["Dependencies"] for the entry whose "Name" (or property
identifying it) equals "BTCPayServer", then set that entry's "Condition" to ">=
1.0.0 && < 2.0.0"; update the code around the unsupportedManifest variable
(reading ManifestInfoJson) to find the correct dependency token (e.g., via
LINQ/JToken.Children().First/Single with a predicate) and modify its "Condition"
rather than indexing [0].
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 689d4d33-a300-4385-98e7-9c44effb5165
📒 Files selected for processing (28)
PluginBuilder.Tests/AdminTests/AdminBTCPayCompatibilityUITests.csPluginBuilder.Tests/BTCPayCompatibilityTests.csPluginBuilder.Tests/PluginTests/BuildUITests.csPluginBuilder.Tests/PluginTests/ImportReviewUITests.csPluginBuilder.Tests/PluginTests/PluginCleanupTests.csPluginBuilder.Tests/PluginTests/VideoUITests.csPluginBuilder.Tests/PublicTests/PublicDirectoryUITests.csPluginBuilder.Tests/UnitTest1.csPluginBuilder/APIModels/PublishedVersion.csPluginBuilder/Controllers/AdminController.csPluginBuilder/Controllers/ApiController.csPluginBuilder/Controllers/PluginController.csPluginBuilder/Data/Scripts/19.BTCPayCompatibility.sqlPluginBuilder/PluginManifest.csPluginBuilder/PluginVersion.csPluginBuilder/Services/BuildService.csPluginBuilder/Util/Extensions/NpgsqlConnectionExtensions.csPluginBuilder/ViewModels/Admin/AdminPluginViewModel.csPluginBuilder/ViewModels/Admin/PluginEditViewModel.csPluginBuilder/ViewModels/BuildViewModel.csPluginBuilder/ViewModels/PluginOwnersTableViewModel.csPluginBuilder/ViewModels/Shared/BTCPayCompatibilityModalViewModel.csPluginBuilder/Views/Admin/ListPlugins.cshtmlPluginBuilder/Views/Admin/PluginEdit.cshtmlPluginBuilder/Views/Plugin/Build.cshtmlPluginBuilder/Views/Shared/_BTCPayCompatibilityModal.cshtmlPluginBuilder/Views/Shared/_PluginOwnersTable.cshtmlPluginBuilder/wwwroot/swagger/v1/swagger.json
✅ Files skipped from review due to trivial changes (9)
- PluginBuilder.Tests/PluginTests/ImportReviewUITests.cs
- PluginBuilder/ViewModels/PluginOwnersTableViewModel.cs
- PluginBuilder.Tests/PublicTests/PublicDirectoryUITests.cs
- PluginBuilder/ViewModels/Admin/AdminPluginViewModel.cs
- PluginBuilder/ViewModels/BuildViewModel.cs
- PluginBuilder/ViewModels/Shared/BTCPayCompatibilityModalViewModel.cs
- PluginBuilder.Tests/PluginTests/PluginCleanupTests.cs
- PluginBuilder/Views/Shared/_BTCPayCompatibilityModal.cshtml
- PluginBuilder/APIModels/PublishedVersion.cs
🚧 Files skipped from review as they are similar to previous changes (8)
- PluginBuilder/Views/Shared/_PluginOwnersTable.cshtml
- PluginBuilder/PluginVersion.cs
- PluginBuilder/Services/BuildService.cs
- PluginBuilder/wwwroot/swagger/v1/swagger.json
- PluginBuilder/Util/Extensions/NpgsqlConnectionExtensions.cs
- PluginBuilder.Tests/AdminTests/AdminBTCPayCompatibilityUITests.cs
- PluginBuilder/ViewModels/Admin/PluginEditViewModel.cs
- PluginBuilder/Data/Scripts/19.BTCPayCompatibility.sql
Comment on lines
+537
to
+545
| var row = await conn.QueryFirstOrDefaultAsync<(int[] btcpay_min_ver, int[]? btcpay_max_ver, string manifest_info)?>( | ||
| """ | ||
| SELECT v.btcpay_min_ver, v.btcpay_max_ver, b.manifest_info | ||
| FROM versions v | ||
| JOIN builds b ON b.plugin_slug = v.plugin_slug AND b.id = v.build_id | ||
| WHERE v.plugin_slug = @pluginSlug AND v.build_id = @buildId | ||
| LIMIT 1 | ||
| """, | ||
| new { pluginSlug = pluginSlug.ToString(), buildId }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 'QueryFirstOrDefaultAsync<\(int\[\] btcpay_min_ver|btcpay_min_ver is \{ Length: > 0 \}' PluginBuilder/Controllers/PluginController.cs
rg -n -C3 'btcpay_(min|max)_ver' -g '*.cs' -g '*.sql'Repository: btcpayserver/btcpayserver-plugin-builder
Length of output: 42673
Stop selecting unused BTCPay compatibility columns in this POST query.
Line 537 maps v.btcpay_min_ver as non-nullable int[], but the GET path defends against null values by treating both columns as nullable. If legacy rows have null min_ver compatibility data, Dapper deserialization fails before any validation runs. Since this endpoint only needs manifest_info, remove the unused arrays from the query.
🩹 Proposed fix
- var row = await conn.QueryFirstOrDefaultAsync<(int[] btcpay_min_ver, int[]? btcpay_max_ver, string manifest_info)?>(
+ var row = await conn.QueryFirstOrDefaultAsync<(string manifest_info)?>(
"""
- SELECT v.btcpay_min_ver, v.btcpay_max_ver, b.manifest_info
+ SELECT b.manifest_info
FROM versions v
JOIN builds b ON b.plugin_slug = v.plugin_slug AND b.id = v.build_id
WHERE v.plugin_slug = `@pluginSlug` AND v.build_id = `@buildId`
LIMIT 1
""",
new { pluginSlug = pluginSlug.ToString(), buildId });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var row = await conn.QueryFirstOrDefaultAsync<(int[] btcpay_min_ver, int[]? btcpay_max_ver, string manifest_info)?>( | |
| """ | |
| SELECT v.btcpay_min_ver, v.btcpay_max_ver, b.manifest_info | |
| FROM versions v | |
| JOIN builds b ON b.plugin_slug = v.plugin_slug AND b.id = v.build_id | |
| WHERE v.plugin_slug = @pluginSlug AND v.build_id = @buildId | |
| LIMIT 1 | |
| """, | |
| new { pluginSlug = pluginSlug.ToString(), buildId }); | |
| var row = await conn.QueryFirstOrDefaultAsync<(string manifest_info)?>( | |
| """ | |
| SELECT b.manifest_info | |
| FROM versions v | |
| JOIN builds b ON b.plugin_slug = v.plugin_slug AND b.id = v.build_id | |
| WHERE v.plugin_slug = `@pluginSlug` AND v.build_id = `@buildId` | |
| LIMIT 1 | |
| """, | |
| new { pluginSlug = pluginSlug.ToString(), buildId }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@PluginBuilder/Controllers/PluginController.cs` around lines 537 - 545, The
query in PluginController.cs selects v.btcpay_min_ver and v.btcpay_max_ver
(mapped as int[] btcpay_min_ver, int[]? btcpay_max_ver) causing Dapper to fail
on NULL legacy rows even though this endpoint only needs manifest_info; update
the SQL passed to conn.QueryFirstOrDefaultAsync (and the mapped tuple type used
for row) to select only b.manifest_info so that the btcpay_* columns are not
requested or mapped, leaving the rest of the method (row usage/validation of
manifest_info) unchanged.
bb3115f to
c161ab5
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
PluginBuilder/Views/Plugin/Build.cshtml (1)
157-190:⚠️ Potential issue | 🟠 MajorCompatibility row not visible when no range is set.
The row only renders when
BTCPayMinVersionorBTCPayMaxVersionis non-empty. For builds with no manifest BTCPay condition and no existing override, the modal partial is rendered (Lines 120-123) but there's no trigger to open it. Owners cannot add compatibility from the build page in this scenario.Consider updating the condition to also render when
CanEditBTCPayCompatibilityis true:- `@if` (Model.Version != null && (!string.IsNullOrEmpty(Model.BTCPayMinVersion) || !string.IsNullOrEmpty(Model.BTCPayMaxVersion))) + `@if` (Model.Version != null && (Model.CanEditBTCPayCompatibility || !string.IsNullOrEmpty(Model.BTCPayMinVersion) || !string.IsNullOrEmpty(Model.BTCPayMaxVersion)))Also handle the case where
BTCPayMinVersionis empty in the display (similar to how max is handled on Lines 166-173).
,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PluginBuilder/Views/Plugin/Build.cshtml` around lines 157 - 190, The compatibility row only renders when BTCPayMinVersion or BTCPayMaxVersion is set, so owners can't open the modal when no range exists; update the top-level condition that checks Model.Version to also include Model.CanEditBTCPayCompatibility so the row and the edit button (compatibility-edit-trigger / data-bs-target="#btcpayCompatibilityModal") are rendered whenever the user can edit; additionally, mirror the existing max-version display logic for the min-version (Model.BTCPayMinVersion) so that if it's empty you show a muted "None" placeholder instead of rendering an empty <code> element.
🧹 Nitpick comments (3)
PluginBuilder.Tests/AdminTests/AdminBTCPayCompatibilityUITests.cs (1)
220-221: Pre-release context relies on implicit version row creation.When
preReleaseis true,SetVersionBuildis not called (Line 220-221 only runs for!preRelease). This relies onCreateAndBuildPluginAsyncimplicitly creating a version row. While the test passes, explicitly callingSetVersionBuild(..., preRelease: true)would make the test setup clearer and ensure the prerelease-specific code path is reliably exercised.💡 Suggested fix
var manifest = PluginManifest.Parse(manifestInfoJson); - if (!preRelease) - await conn.SetVersionBuild(fullBuildId, manifest.Version, manifest.BTCPayMinVersion, manifest.BTCPayMaxVersion, false); + await conn.SetVersionBuild(fullBuildId, manifest.Version, manifest.BTCPayMinVersion, manifest.BTCPayMaxVersion, preRelease);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PluginBuilder.Tests/AdminTests/AdminBTCPayCompatibilityUITests.cs` around lines 220 - 221, The test currently skips calling SetVersionBuild when preRelease is true, relying on CreateAndBuildPluginAsync to implicitly create the version row; change the setup to explicitly call SetVersionBuild(fullBuildId, manifest.Version, manifest.BTCPayMinVersion, manifest.BTCPayMaxVersion, true) when preRelease is true so the prerelease code path is exercised; locate the branch around the preRelease conditional in AdminBTCPayCompatibilityUITests.cs and add the explicit SetVersionBuild call (or convert the existing conditional to always call SetVersionBuild with the last argument set to preRelease) to make the test intent clear and deterministic.PluginBuilder/Controllers/AdminController.cs (1)
671-688: Consider using strict BTCPay condition parsing.
TryGetBtcPayDependencyConditionextracts the rawConditionstring from the manifest. For consistency with how the admin UI indicates whether reset-to-manifest is supported, consider also checking if the condition can be parsed in strict mode (similar to howUpdateVersionBtcPayCompatibilityusesstrictBTCPayVersionCondition: true).This would allow the UI to show whether the condition is parseable vs just displaying the raw string.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PluginBuilder/Controllers/AdminController.cs` around lines 671 - 688, TryGetBtcPayDependencyCondition currently returns the raw "Condition" string; change it to validate/parse that condition using the same strict BTCPay condition parser used by UpdateVersionBtcPayCompatibility (i.e., with strictBTCPayVersionCondition: true) and only return the condition string if parsing succeeds (otherwise return null). Locate TryGetBtcPayDependencyCondition, extract the "Condition" value as before, then call the existing BTCPay condition parsing/validation helper used by UpdateVersionBtcPayCompatibility in strict mode; if the parser accepts the condition, return it, else return null so the UI can distinguish parseable vs raw conditions.PluginBuilder/Views/Shared/_PluginOwnersTable.cshtml (1)
13-13: Thetabroute parameter is redundant.Per the context snippets, the
TransferPrimaryOwner,AddOwner, andRemoveOwnercontroller actions do not accept atabparameter in their signatures. Each action always redirects with the hardcodedtab = PluginEditTabs.Owners. TheModel.Tabvalues passed here are silently ignored by ASP.NET MVC model binding.This isn't a bug, but it adds unnecessary complexity. Consider either:
- Removing these redundant
tab = Model.Tabadditions, or- Updating the controller actions to accept and use the
tabparameter if preserving tab context is desired for future use cases.Also applies to: 51-51, 67-67, 85-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PluginBuilder/Views/Shared/_PluginOwnersTable.cshtml` at line 13, Remove the redundant asp-route-tab="@Model.Tab" parameters from the view helpers that call the owner actions (the links/forms that invoke TransferPrimaryOwner, AddOwner, and RemoveOwner) since those controller actions don't accept a tab parameter and always redirect with tab = PluginEditTabs.Owners; locate the tag helpers in _PluginOwnersTable.cshtml that include asp-route-tab="@Model.Tab" (three occurrences) and delete that attribute so MVC won't pass an ignored route value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@PluginBuilder/Views/Plugin/Build.cshtml`:
- Around line 157-190: The compatibility row only renders when BTCPayMinVersion
or BTCPayMaxVersion is set, so owners can't open the modal when no range exists;
update the top-level condition that checks Model.Version to also include
Model.CanEditBTCPayCompatibility so the row and the edit button
(compatibility-edit-trigger / data-bs-target="#btcpayCompatibilityModal") are
rendered whenever the user can edit; additionally, mirror the existing
max-version display logic for the min-version (Model.BTCPayMinVersion) so that
if it's empty you show a muted "None" placeholder instead of rendering an empty
<code> element.
---
Nitpick comments:
In `@PluginBuilder.Tests/AdminTests/AdminBTCPayCompatibilityUITests.cs`:
- Around line 220-221: The test currently skips calling SetVersionBuild when
preRelease is true, relying on CreateAndBuildPluginAsync to implicitly create
the version row; change the setup to explicitly call
SetVersionBuild(fullBuildId, manifest.Version, manifest.BTCPayMinVersion,
manifest.BTCPayMaxVersion, true) when preRelease is true so the prerelease code
path is exercised; locate the branch around the preRelease conditional in
AdminBTCPayCompatibilityUITests.cs and add the explicit SetVersionBuild call (or
convert the existing conditional to always call SetVersionBuild with the last
argument set to preRelease) to make the test intent clear and deterministic.
In `@PluginBuilder/Controllers/AdminController.cs`:
- Around line 671-688: TryGetBtcPayDependencyCondition currently returns the raw
"Condition" string; change it to validate/parse that condition using the same
strict BTCPay condition parser used by UpdateVersionBtcPayCompatibility (i.e.,
with strictBTCPayVersionCondition: true) and only return the condition string if
parsing succeeds (otherwise return null). Locate
TryGetBtcPayDependencyCondition, extract the "Condition" value as before, then
call the existing BTCPay condition parsing/validation helper used by
UpdateVersionBtcPayCompatibility in strict mode; if the parser accepts the
condition, return it, else return null so the UI can distinguish parseable vs
raw conditions.
In `@PluginBuilder/Views/Shared/_PluginOwnersTable.cshtml`:
- Line 13: Remove the redundant asp-route-tab="@Model.Tab" parameters from the
view helpers that call the owner actions (the links/forms that invoke
TransferPrimaryOwner, AddOwner, and RemoveOwner) since those controller actions
don't accept a tab parameter and always redirect with tab =
PluginEditTabs.Owners; locate the tag helpers in _PluginOwnersTable.cshtml that
include asp-route-tab="@Model.Tab" (three occurrences) and delete that attribute
so MVC won't pass an ignored route value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8b2897f3-4bfc-4baa-8b8d-0745fe7685c0
📒 Files selected for processing (16)
PluginBuilder.Tests/AdminTests/AdminBTCPayCompatibilityUITests.csPluginBuilder.Tests/PluginTests/BuildUITests.csPluginBuilder.Tests/PluginTests/ImportReviewUITests.csPluginBuilder.Tests/PluginTests/VideoUITests.csPluginBuilder/Controllers/AdminController.csPluginBuilder/Controllers/PluginController.csPluginBuilder/ViewModels/Admin/AdminPluginViewModel.csPluginBuilder/ViewModels/Admin/PluginEditViewModel.csPluginBuilder/ViewModels/BuildViewModel.csPluginBuilder/ViewModels/PluginOwnersTableViewModel.csPluginBuilder/ViewModels/Shared/BTCPayCompatibilityModalViewModel.csPluginBuilder/Views/Admin/ListPlugins.cshtmlPluginBuilder/Views/Admin/PluginEdit.cshtmlPluginBuilder/Views/Plugin/Build.cshtmlPluginBuilder/Views/Shared/_BTCPayCompatibilityModal.cshtmlPluginBuilder/Views/Shared/_PluginOwnersTable.cshtml
✅ Files skipped from review due to trivial changes (6)
- PluginBuilder.Tests/PluginTests/ImportReviewUITests.cs
- PluginBuilder/ViewModels/PluginOwnersTableViewModel.cs
- PluginBuilder/ViewModels/Admin/AdminPluginViewModel.cs
- PluginBuilder/ViewModels/BuildViewModel.cs
- PluginBuilder/ViewModels/Shared/BTCPayCompatibilityModalViewModel.cs
- PluginBuilder/Controllers/PluginController.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- PluginBuilder.Tests/PluginTests/VideoUITests.cs
- PluginBuilder/Views/Shared/_BTCPayCompatibilityModal.cshtml
- PluginBuilder/Views/Admin/PluginEdit.cshtml
rockstardev
approved these changes
Mar 21, 2026
Member
rockstardev
left a comment
rockstardev
left a comment
There was a problem hiding this comment.
@thgO-O great job on this PR. The only nit I can find with user testing is that in BTCPay Server core we will want a PR where we display max version dependencies in UI.
max.version.in.plugin.builder.mp4
Ready to merge and deploy imo
This was referenced Mar 23, 2026
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.
Description
This PR adds BTCPay compatibility management to plugin versions.
It introduces BTCPay max range support, persists the effective compatibility in the versions table, and applies it to the public catalog and update endpoints.
It also adds UI support for editing compatibility from:
Compatibility overrides now persist across new builds until reset to the manifest defaults.
btcpay-max-version.mov