Feature/ad expiration#499
Merged
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. WalkthroughAdds nullable UTC display window fields and helper to Partner; enforces them in ad selection and partner lists; exposes and validates fields in create/edit/admin flows; adds EF migration and UI changes. Centralizes HTML email builders, preview UI and tests; adds “actively looking” flags; removes two task docs. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client (UI)
participant PartnerSvc as Partner Service
participant AdProv as AdProvider
participant DB as Database
UI->>PartnerSvc: POST/PUT partner { DisplayStartUtc?, DisplayEndUtc? }
PartnerSvc->>DB: INSERT/UPDATE Partner (DisplayStartUtc/DisplayEndUtc)
DB-->>PartnerSvc: OK
UI->>AdProv: GET /ads
AdProv->>AdProv: utcNow = DateTime.UtcNow
AdProv->>DB: SELECT Partners WHERE (DisplayStartUtc IS NULL OR DisplayStartUtc <= utcNow) AND (DisplayEndUtc IS NULL OR DisplayEndUtc >= utcNow)
DB-->>AdProv: partners within window
AdProv-->>UI: Return filtered ads
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/web/Jordnaer/Features/Partners/PartnerUserService.cs`:
- Around line 35-36: Add validation to ensure DisplayStartUtc is strictly before
DisplayEndUtc when both are provided: inside CreatePartnerAccountAsync (or the
partner request model validation) check if request.DisplayStartUtc.HasValue &&
request.DisplayEndUtc.HasValue and if request.DisplayStartUtc.Value >=
request.DisplayEndUtc.Value reject the request (e.g., return a
BadRequest/validation error or throw a domain validation exception) with a clear
message; this prevents IsWithinDisplayWindow from silently making the partner
invisible and surfaces the misconfiguration to the caller.
In `@src/web/Jordnaer/Pages/Backoffice/CreatePartnerPage.razor`:
- Around line 96-106: The end-date from MudDatePicker (_model.DisplayEndDate) is
at midnight which makes DisplayEndUtc cut visibility a day early; update the
mapping that sets DisplayEndUtc (or wherever utcNow <= DisplayEndUtc is
evaluated) to treat the selected end date as end-of-day by adding time
23:59:59.999 (or the equivalent end-of-day Instant) when
_model.DisplayEndDate.HasValue before converting to UTC, so the comparison
utcNow <= DisplayEndUtc keeps the partner visible for the entire selected day.
Ensure you update the code paths that persist or use DisplayEndUtc (and any
validation) so the change is consistently applied.
In `@tasks/11-email-content-improvements.md`:
- Line 341: Update the example path string
"tests/web/Jordnaer.Tests/bin/Debug/net10.0/email-previews/" to use a valid
target framework or a generic placeholder; replace "net10.0" with the correct
target (e.g., "net8.0" or "net9.0") or "net<version>" so the preview path is
accurate and not misleading.
In `@TODO.md`:
- Line 9: The "Actively looking" section header in TODO.md is missing acceptance
test items; add a short checklist under the "Actively looking" heading that
mirrors the style used in "Active ad dates" and includes concrete acceptance
tests such as: user search with filter (e.g., location/skills), group search
with filter, profile edit acceptance criteria, and group create/edit acceptance
criteria so the section is complete and testable.
🧹 Nitpick comments (3)
src/web/Jordnaer/Pages/Footer/Partners.razor (1)
45-56: Client-side filtering after fetching all partners.
GetAllPartnersAsync()loads all partners, then filters in-memory forHasPartnerCardandIsWithinDisplayWindow. This works for a small dataset but consider pushing the display window filter to the query level (similar toAdProvider.cs) if the partner count grows significantly.tasks/11-email-content-improvements.md (2)
261-262: Consider documenting the NUnit dependency.The code uses
TestContext.CurrentContext, which is NUnit-specific. If the project uses a different test framework (xUnit, MSTest), this implementation won't work. Consider adding a note in the documentation that this helper requires NUnit.
254-284: Consider adding a cleanup strategy for preview files.The
EmailPreviewHelpercreates timestamped HTML files but doesn't include any cleanup mechanism. Over time, these files could accumulate in the output directory. Consider documenting a cleanup strategy, such as:
- Deleting files older than a certain age
- Keeping only the N most recent previews per email type
- Running cleanup at the start of test runs
This is a nice-to-have improvement for maintainability but not critical for the initial implementation.
github-project-automation
Bot
moved this from Todo
to In Progress
in Jordnaer Community Website
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/web/Jordnaer/Pages/Backoffice/PartnerDetailPage.razor`:
- Around line 164-179: The MudItem components (MudItem) are being used inside a
MudStack, so their responsive attributes (xs/md) won't apply; wrap the pair of
MudItem elements (the block rendering Created and conditional LastUpdateUtc) in
a MudGrid container (MudGrid) and move MudItem children directly under that
MudGrid (or replace MudItem with plain markup if you prefer stacking behavior),
ensuring the MudGrid encloses both MudItem entries so xs="12" md="6" takes
effect.
🧹 Nitpick comments (2)
src/web/Jordnaer/Pages/Backoffice/PartnerDetailPage.razor (1)
63-64:DataAnnotationsValidatorhas no effect sinceEditPartnerModellacks any data annotations.The
OnValidSubmitcallback will always fire because the model class at lines 629–640 has no[Required],[Url],[MaxLength], or custom validation attributes. Consider either adding annotations toEditPartnerModel(e.g.,[Url]on link fields, a customIValidatableObjectimplementation for the date range check) to give inline validation feedback, or removing theDataAnnotationsValidatorif server-side validation alone is sufficient.src/web/Jordnaer/Features/Partners/PartnerService.cs (1)
833-858: URL and hex-color validation is duplicated fromUploadPendingChangesAsync.The URL validation block (lines 833–850) and hex-color regex check (lines 852–858) are near-identical copies of the logic in
UploadPendingChangesAsync(lines 413–448). Consider extracting these into private helper methods (e.g.,ValidateUrl,ValidateHexColor) to keep them DRY and ensure any future changes to the validation rules apply consistently.Additionally, the regex on line 854 (
Regex.IsMatch(...)) allocates a newRegexinternally on every call. Astatic partialgenerated regex or a pre-compiledstatic readonly Regexfield would be more efficient.Example: extract validation helpers and use a static regex
public class PartnerService( IDbContextFactory<JordnaerDbContext> contextFactory, ... { + [GeneratedRegex("^#[0-9A-Fa-f]{6}$")] + private static partial Regex HexColorRegex(); + + private static OneOf<string, Error<string>> ValidateUrl(string? url, string fieldName) + { + if (string.IsNullOrWhiteSpace(url)) + return string.Empty; // no URL provided, OK + + var trimmed = url.Trim(); + if (!Uri.TryCreate(trimmed, UriKind.Absolute, out var uri) || + (uri.Scheme != Uri.UriSchemeHttp && uri.Scheme != Uri.UriSchemeHttps)) + { + return new Error<string>($"Ugyldig {fieldName} URL"); + } + return trimmed; + } + + private static OneOf<string, Error<string>> ValidateHexColor(string? color) + { + if (string.IsNullOrWhiteSpace(color)) + return string.Empty; + + var trimmed = color.Trim(); + if (!HexColorRegex().IsMatch(trimmed)) + return new Error<string>("Ugyldig farve. Brug hex format som `#FFFFFF`"); + return trimmed; + }
improve emails and add preview system
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/web/Jordnaer/Features/Email/EmailContentBuilder.cs`:
- Around line 97-114: The preheader text is double-HTML-encoded because
GroupPostNotification passes encodedAuthorName (already HtmlEncoded) into
EmailTemplate.Wrap which HtmlEncodes preheaderText again; change the preheader
argument to use the raw authorName instead of encodedAuthorName (keep using
encodedAuthorName inside the body/blockquote and Button call), i.e. pass
authorName to the preheaderText parameter when calling EmailTemplate.Wrap so
EmailTemplate.Wrap can perform the single intended HtmlEncode.
- Around line 116-125: The preheader is being double-HTML-encoded:
MembershipRequest currently passes encodedGroupName (produced by
WebUtility.HtmlEncode) into EmailTemplate.Wrap's preheaderText, which will
HTML-encode again; fix this by passing the raw groupName to the preheaderText
argument (keep HtmlEncode for the in-body display variable used in the HTML
markup), i.e., retain encodedGroupName for the <h4> display but use groupName
for the preheaderText parameter to avoid double-encoding.
In `@src/web/Jordnaer/Features/Email/EmailService.cs`:
- Line 60: The Subject assignment in EmailService is HTML-encoding senderName
which causes subjects like "Partner & Co" to appear garbled; change the
Subject construction in the method that sets Subject to use the raw senderName
interpolation (remove WebUtility.HtmlEncode) to match the other subject lines
(see similar usages around the other subject assignments) so the plain-text
email subject shows the actual characters.
In `@src/web/Jordnaer/Pages/Backoffice/PartnerDetailPage.razor`:
- Around line 773-797: Add a confirmation dialog before performing the
destructive action in ResendWelcomeEmailAsync: use IDialogService to show a
confirmation dialog when ResendWelcomeEmailAsync is invoked, and only call
PartnerUserService.ResendWelcomeEmailAsync(_partner.UserId) if the user
confirms; if the dialog is cancelled, return early without changing _processing.
Ensure you still set _processing true only while the operation runs and reset it
in the finally block, and preserve existing logging and Snackbar result handling
in the success/error branches.
- Around line 634-664: The Url attributes on PartnerPageLink and AdLink reject
empty strings, so when MudTextField binds an empty string the form fails; update
the EditPartnerModel by replacing the auto-properties PartnerPageLink and AdLink
with explicit properties whose setter coerces string.Empty to null (e.g., set
backingField = string.IsNullOrWhiteSpace(value) ? null : value) so cleared
inputs become null and pass validation, and ensure any other code reading these
properties handles nulls accordingly.
- Around line 737-743: Add a brief inline comment above the
DisplayStartUtc/DisplayEndUtc assignments explaining the asymmetry: that
MudDatePicker returns midnight for picked dates so DisplayStartUtc uses
DateTime.SpecifyKind to preserve the start-of-day (00:00:00 UTC) while
DisplayEndUtc intentionally normalizes to end-of-day using
.Date.AddDays(1).AddTicks(-1) to produce 23:59:59.9999999 UTC (matching SQL
Server datetime2 precision), so future maintainers understand the intent; place
the comment near the _editModel.DisplayStartDate and _editModel.DisplayEndDate
handling and reference the MudDatePicker behavior and AddTicks(-1) rationale.
🧹 Nitpick comments (2)
src/web/Jordnaer/Pages/Backoffice/PartnerDetailPage.razor (1)
747-760:LoadPartnerDataAsync()is called even on failure paths.Line 760 calls
await LoadPartnerDataAsync()unconditionally after theresult.Switch(...). On failure (notFound, error), this triggers a redundant network call. Consider moving the reload inside the success branch.Proposed fix
result.Switch( success => { Snackbar.Add("Partner opdateret!", Severity.Success); _isEditing = false; }, notFound => { Snackbar.Add("Partner ikke fundet", Severity.Warning); }, error => Snackbar.Add(error.Value, Severity.Error) ); - await LoadPartnerDataAsync(); + if (result.IsT0) // success + { + await LoadPartnerDataAsync(); + }src/web/Jordnaer/Features/Email/EmailTemplate.cs (1)
6-68:FooterSignatureconstant is not used in the actual footer HTML — drift risk.Line 8 defines
FooterSignature = "Mini Møder Teamet"and line 57 hardcodes the same string in the HTML. If one changes independently, tests (which assert against the constant) will pass while the actual email content diverges. Consider interpolating the constant into the template.Proposed fix
- <p style="margin: 0;">Venlig hilsen,<br>Mini Møder Teamet</p> + <p style="margin: 0;">Venlig hilsen,<br>{FooterSignature}</p>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/web/Jordnaer/Pages/Backoffice/PartnerDetailPage.razor`:
- Around line 148-162: The date pickers currently label values as UTC but the
component binds to _editModel.DisplayStartDate and _editModel.DisplayEndDate
which StartEditing converts from UTC to Copenhagen local time and
SaveChangesAsync converts back, so update the MudDatePicker Label texts (for the
MudDatePicker bound to _editModel.DisplayStartDate and the one bound to
_editModel.DisplayEndDate) to indicate Copenhagen/local time (e.g., change
"Startdato (UTC)" and "Slutdato (UTC)" to "Startdato (København, lokal tid)" and
"Slutdato (København, lokal tid)") so admins aren't misled; no other logic
changes are needed since StartEditing and SaveChangesAsync handle the
conversions.
🧹 Nitpick comments (1)
src/web/Jordnaer/Pages/Backoffice/PartnerDetailPage.razor (1)
631-642: Silent UTC fallback could cause subtle date shifts.If neither timezone ID is found (unlikely but possible in unusual container environments), dates will silently be treated as UTC instead of Copenhagen time, shifting display windows by 1–2 hours. Consider throwing an exception at startup rather than silently degrading.
Proposed fix
private static TimeZoneInfo GetCopenhagenTimeZone() { if (TimeZoneInfo.TryFindSystemTimeZoneById("Romance Standard Time", out var tz)) return tz; if (TimeZoneInfo.TryFindSystemTimeZoneById("Europe/Copenhagen", out tz)) return tz; - return TimeZoneInfo.Utc; + throw new InvalidOperationException( + "Could not find Copenhagen timezone. Ensure the runtime has timezone data installed."); }
github-project-automation
Bot
moved this from In Progress
to Done
in Jordnaer Community Website
Merged
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
1 participant
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.
No description provided.