feat: data export, in-app feedback, and EN/DE localization (polish batch) - #32
Conversation
GetTestExport(userId) returns every test for the user — past and future, graded or not — with subject names resolved and enum values rendered to readable text, strictly scoped to the given user id. TestExporter.ToCsv serialises the rows as RFC 4180 CSV (CRLF, quoted fields) with formula-injection mitigation on the user-controlled Title and Subject columns. Tests cover user isolation, inclusion of past graded tests, CSV escaping, and injection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GET /api/export?format=csv|json returns the authenticated user's tests as a downloadable file (CSV default, JSON optional), resolving the user from their own claims so the export can never include another user's rows — nDSG / GDPR data portability. The CSV carries a UTF-8 BOM so spreadsheets render accented characters. Settings gains a "Data" block with CSV/JSON download links via a plain HTTP endpoint, avoiding a file download over the Blazor SignalR circuit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
New Feedback table for in-app feedback / feature requests: Id, UserId (FK to Users, cascade delete), Message (varchar 4000), Category and Status enums stored as ints (consistent with Test.Volume), and a CreatedAt timestamp indexed for the newest-first admin listing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
FeedbackManagement.Submit validates and trims the message, enforces the 4000-char column cap and a defined category, then persists an Open feedback row. GetAll returns a newest-first projection (with submitter name/email) for the admin listing. Authorization is left to the web layer — the engine has no notion of who the admin is. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A /feedback page (linked from Settings) lets any logged-in user submit feedback with a category and message. Submissions go through FeedbackManagement; the page maps validation errors to inline messages and clears the form on success. The full-submissions listing on the same page is gated to the configured admin account (Admin:Email, env-overridable as Admin__Email); a blank value means nobody is admin. AuthenticatedComponentBase now exposes CurrentUserEmail so pages can do this owner check without re-reading the auth state. /feedback is added to the auth-protected path list. The page component is named FeedbackPage to avoid colliding with the WSIST.Engine.Feedback entity type in Razor. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds tests for FeedbackManagement: trimmed/Open persistence, empty and over-long message rejection, undefined-category rejection, and the newest-first GetAll projection with resolved submitter name/email. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sets up ASP.NET Core request localization with English and German UI
cultures while pinning the formatting culture to English, so switching
language never changes how grades/dates are parsed or displayed.
- AddLocalization + RequestLocalizationOptions (supported UI cultures
en/de; default en).
- DbRequestCultureProvider resolves a signed-in user's stored
PreferredLanguage ahead of the cookie and Accept-Language providers,
so the choice persists across logout/login. A null preference falls
through to the browser language (first-login default).
- User.PreferredLanguage column (+ migration) and engine methods to
read/update it.
- /set-language/{culture} endpoint writes the culture cookie, persists
the preference for signed-in users, and does an open-redirect-safe
local redirect back.
- LanguageToggle component (EN / DE) and <html lang> bound to the
current UI culture.
- SharedResource marker for a single shared IStringLocalizer resource.
No user-facing strings are translated yet; that follows per page.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds the shared EN/DE resource files and routes every user-facing string on the authenticated app pages — plus the not-found and error pages — through IStringLocalizer<SharedResource>. The EN/DE language toggle is placed in each page's top nav. - Volume/understanding levels render from Level_* keys (one source for selects, tables and the study "because" text), so the Test enum helpers are no longer shown raw. - Study's composed "because" sentence and score breakdown are built from parameterized resource templates. - Pluralized day/test counts use explicit one/many keys. - German copy is High German with consistent terminology (Prüfung, Fach, Note, Umfang, Verständnis). Landing page and legal pages follow next. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Routes every string on the marketing landing page through IStringLocalizer and adds the EN/DE toggle to its nav. The interactive playground (landing.js) runs client-side and can't use the server localizer, so it carries its own EN/DE table for the level labels, day labels and verdict text, picked from <html lang>. Illustrative sample content (example test titles like "Algebra II — Quadratics") is left as-is; subject-name demo tokens, day/points labels and all prose are localized. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Each legal section is stored as a static, developer-authored HTML block in the resource files and rendered via MarkupString, so inline emphasis and links survive translation while keeping the key count small. The German versions are High German (ß spelling, not Swiss) with proper legal phrasing (DSGVO / Schweizer nDSG). Both legal pages get the EN/DE toggle in their nav. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@coderabbitai review |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThree independent features land together: a Feedback submission system (model, EF migrations, service, admin UI page with status updates); a CSV/JSON test export pipeline (TestExporter with formula-injection mitigation, /api/export endpoint, and Settings UI links); and full EN/DE localization (SharedResource RESX files, DB-stored preferred language, DbRequestCultureProvider, LanguageToggle component, /set-language endpoint, and all UI pages converted to use ChangesFeedback Feature
Test Export Feature
Localization (EN/DE) and Preferred Language
Sequence Diagram(s)sequenceDiagram
participant Browser
participant SetLang as GET /set-language/{culture}
participant TestMgmt as TestManagement
participant DB as Users (MySQL)
participant DbProvider as DbRequestCultureProvider
participant Middleware as UseRequestLocalization
participant Page as Blazor Page
Browser->>SetLang: GET /set-language/de?redirectUri=/home
SetLang->>SetLang: Validate culture (en or de only)
SetLang->>Browser: Set localization cookie (de)
SetLang->>TestMgmt: UpdatePreferredLanguage(userId, "de")
TestMgmt->>DB: UPDATE Users SET PreferredLanguage = "de"
SetLang-->>Browser: Redirect to /home
Note over Browser,Page: Subsequent request with authentication
Browser->>Middleware: GET /home (auth cookie + locale cookie)
Middleware->>DbProvider: DetermineProviderCultureResult(httpContext)
DbProvider->>TestMgmt: GetPreferredLanguageByEmail(email)
TestMgmt->>DB: SELECT PreferredLanguage FROM Users
DB-->>DbProvider: "de"
DbProvider-->>Middleware: ProviderCultureResult("de")
Middleware->>Page: Render with CultureInfo "de"
Page-->>Browser: German UI via IStringLocalizer<SharedResource>
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
WSIST/WSIST.Engine/TestManagement.cs (1)
292-296: ⚡ Quick winMove ordering before
AsEnumerable()to keep sorting in SQL.
OrderBy/ThenBycurrently runs in-memory after materialization. Push ordering to the DB and switch to in-memory only for the helper-based projection.Proposed refactor
- return context - .Tests.Where(t => t.UserId == userId) - .AsEnumerable() - .OrderBy(t => t.DueDate) - .ThenBy(t => t.Title) + return context + .Tests.Where(t => t.UserId == userId) + .OrderBy(t => t.DueDate) + .ThenBy(t => t.Title) + .AsEnumerable() .Select(t => new TestExportRow(🤖 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 `@WSIST/WSIST.Engine/TestManagement.cs` around lines 292 - 296, The LINQ query in the method is performing sorting operations in-memory after materializing data with AsEnumerable(), which is inefficient. Move the OrderBy and ThenBy calls before the AsEnumerable() call so that the sorting happens at the database level via SQL. The corrected order should be: apply the Where filter, then OrderBy DueDate, then ThenBy Title, and only then call AsEnumerable() for any remaining in-memory operations.
🤖 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 `@WSIST/WSIST.Engine/FeedbackManagement.cs`:
- Around line 49-52: The OrderByDescending(f => f.CreatedAt) call in the
Feedbacks query sorts only by creation timestamp, which produces unpredictable
ordering when multiple records have identical CreatedAt values. Add a secondary
sort key using ThenByDescending or ThenBy (with the Id field) after the
OrderByDescending call to ensure deterministic and stable ordering when
timestamps are equal.
In `@WSIST/WSIST.Web/appsettings.json`:
- Around line 9-12: The Admin.Email configuration in appsettings.json contains a
hard-coded personal email address (tim.an.zurich@gmail.com) which poses a
security risk if the Admin__Email environment variable override is not set
during production deployment. Replace the real email address with a placeholder
value such as "admin@example.com" or an empty string that clearly indicates this
must be overridden via environment variables, ensuring that no actual personal
email with admin access is left as the default.
In `@WSIST/WSIST.Web/Components/LanguageToggle.razor`:
- Line 7: The aria-label attribute on the language-toggle div element in
LanguageToggle.razor is hardcoded as "Language" in English, causing non-English
users to receive mixed-language screen-reader output. Replace the hardcoded
"Language" string with a localized resource key or value that will display the
appropriate translated label based on the current UI language. Use your
application's localization service or resource mechanism to bind the aria-label
to the correct translated string.
In `@WSIST/WSIST.Web/Components/Pages/FeedbackPage.razor.cs`:
- Around line 53-59: The ArgumentException catch block in FeedbackPage.razor.cs
incorrectly assumes all ArgumentException cases correspond to empty messages and
maps them all to Feedback_EmptyError, but engine validation can also throw
ArgumentException for over-length messages and invalid category values. Refactor
the catch block to inspect the exception details (message or exception type) and
map each validation failure case to its corresponding localized error message
key. For the empty message case use Feedback_EmptyError, and create or use
separate localization keys for over-length and invalid category errors to ensure
users see accurate error messages.
In `@WSIST/WSIST.Web/Components/Pages/Login.razor`:
- Line 317: The hardcoded German string "WSIST sagt" in the h3 element bypasses
the localizer and will display in German even when the UI is set to English.
Replace the hardcoded "WSIST sagt" string with a localized reference using the
`@Localizer` syntax (similar to how `@Localizer`["Landing_Soon"] is used in the same
line). Create a corresponding localization key for this phrase and apply it
consistently wherever this text appears, including the location noted on line
330.
In `@WSIST/WSIST.Web/DbRequestCultureProvider.cs`:
- Around line 15-31: The DetermineProviderCultureResult method is blocking on
synchronous database I/O by calling GetPreferredLanguageByEmail() synchronously
for every authenticated request in the pipeline. Make this method async by
changing its implementation to use await, then add a new async method
GetPreferredLanguageByEmailAsync in the TestManagement class that uses
FirstOrDefaultAsync() instead of synchronous database operations, and update the
call in DetermineProviderCultureResult to await this new async variant instead
of the synchronous GetPreferredLanguageByEmail method.
In `@WSIST/WSIST.Web/Resources/SharedResource.de.resx`:
- Around line 169-170: The Settings_ExportSub value in the German resource file
contains an incomplete German sentence ending with "einschließlich vergangener
benoteter." which lacks a noun object to be grammatically correct. Update the
value attribute of the Settings_ExportSub data element to complete the German
phrase by adding the appropriate noun (such as "Prüfungen") after "benoteter" so
the sentence reads properly and makes sense to German-speaking users.
In `@WSIST/WSIST.Web/wwwroot/app.css`:
- Around line 1147-1152: In the `.feedback-item-message` CSS class, replace the
deprecated `word-break: break-word` property with `overflow-wrap: anywhere`.
This is the modern standard-compliant approach for handling word overflow and
text wrapping. Remove the deprecated property entirely and add the new
overflow-wrap property in its place to maintain the desired text wrapping
behavior.
---
Nitpick comments:
In `@WSIST/WSIST.Engine/TestManagement.cs`:
- Around line 292-296: The LINQ query in the method is performing sorting
operations in-memory after materializing data with AsEnumerable(), which is
inefficient. Move the OrderBy and ThenBy calls before the AsEnumerable() call so
that the sorting happens at the database level via SQL. The corrected order
should be: apply the Where filter, then OrderBy DueDate, then ThenBy Title, and
only then call AsEnumerable() for any remaining in-memory operations.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c3388268-07cf-4063-a896-ace63aa4674a
📒 Files selected for processing (37)
WSIST/WSIST.Engine/Feedback.csWSIST/WSIST.Engine/FeedbackManagement.csWSIST/WSIST.Engine/Migrations/20260617062848_AddFeedbackTable.Designer.csWSIST/WSIST.Engine/Migrations/20260617062848_AddFeedbackTable.csWSIST/WSIST.Engine/Migrations/20260617064345_AddUserPreferredLanguage.Designer.csWSIST/WSIST.Engine/Migrations/20260617064345_AddUserPreferredLanguage.csWSIST/WSIST.Engine/Migrations/WsistContextModelSnapshot.csWSIST/WSIST.Engine/TestExporter.csWSIST/WSIST.Engine/TestManagement.csWSIST/WSIST.Engine/User.csWSIST/WSIST.Engine/WsistContext.csWSIST/WSIST.UnitTests/UnitTests.csWSIST/WSIST.Web/Components/App.razorWSIST/WSIST.Web/Components/LanguageToggle.razorWSIST/WSIST.Web/Components/Pages/AuthenticatedComponentBase.csWSIST/WSIST.Web/Components/Pages/Error.razorWSIST/WSIST.Web/Components/Pages/FeedbackPage.razorWSIST/WSIST.Web/Components/Pages/FeedbackPage.razor.csWSIST/WSIST.Web/Components/Pages/Home.razorWSIST/WSIST.Web/Components/Pages/Home.razor.csWSIST/WSIST.Web/Components/Pages/Login.razorWSIST/WSIST.Web/Components/Pages/NotFound.razorWSIST/WSIST.Web/Components/Pages/Privacy.razorWSIST/WSIST.Web/Components/Pages/Settings.razorWSIST/WSIST.Web/Components/Pages/Settings.razor.csWSIST/WSIST.Web/Components/Pages/Study.razorWSIST/WSIST.Web/Components/Pages/Study.razor.csWSIST/WSIST.Web/Components/Pages/Terms.razorWSIST/WSIST.Web/Components/_Imports.razorWSIST/WSIST.Web/DbRequestCultureProvider.csWSIST/WSIST.Web/Program.csWSIST/WSIST.Web/Resources/SharedResource.de.resxWSIST/WSIST.Web/Resources/SharedResource.resxWSIST/WSIST.Web/SharedResource.csWSIST/WSIST.Web/appsettings.jsonWSIST/WSIST.Web/wwwroot/app.cssWSIST/WSIST.Web/wwwroot/landing.js
- Blank the committed Admin:Email default (was a real mailbox); the
admin email now comes from Admin__Email / appsettings.Development.json.
A blank value means nobody is admin.
- Make DbRequestCultureProvider async (GetPreferredLanguageByEmailAsync
via FirstOrDefaultAsync) so the request-localization pipeline no longer
blocks on synchronous DB I/O for every authenticated request.
- Deterministic admin feedback ordering: tie-break on Id after CreatedAt.
- Map non-empty feedback validation failures to a generic localized
error instead of always showing the empty-message text.
- Localize the language-toggle aria-label.
- Localize the last hardcoded landing strings ("WSIST sagt" and the
manifesto attribution).
- Complete the German export subtitle sentence.
- Replace deprecated word-break: break-word with overflow-wrap: anywhere.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- FeedbackManagement.UpdateStatus(id, status) lets the admin move a submission through Open/Reviewed/Closed; the listing now renders a per-item status dropdown that saves on change. Tests cover the update and the missing-row case. - The WSIST logo/title in the Settings, Study, and Feedback top navs is now a link to "/" (was a non-clickable div). Note: feedback is still DB-only — there is no reply-to-user channel. Replying to a submitter would need email/notification delivery, which isn't built yet. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds FeedbackManagement.GetForUser(userId) and a "Your feedback" section on /feedback, visible to every signed-in user, listing their own submissions (newest first) with category, status, date and message — read-only; only the admin can change status. Refreshes after submitting and after an admin status change. Test covers per-user isolation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
WSIST/WSIST.Engine/TestManagement.cs (1)
301-303: ⚡ Quick winPush export ordering to SQL before materialization.
Line 301 materializes early, so sorting runs in memory. Move
AsEnumerable()afterOrderBy/ThenByso DB handles ordering.♻️ Proposed refactor
return context .Tests.Where(t => t.UserId == userId) - .AsEnumerable() .OrderBy(t => t.DueDate) .ThenBy(t => t.Title) + .AsEnumerable() .Select(t => new TestExportRow( t.Title, subjects.TryGetValue(t.Subject, out var name) ? name : t.Subject.ToString(),🤖 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 `@WSIST/WSIST.Engine/TestManagement.cs` around lines 301 - 303, The AsEnumerable() call is materializing the data into memory before the OrderBy and ThenBy operations, causing sorting to happen in memory instead of at the database level. Move the AsEnumerable() call to after the OrderBy(t => t.DueDate) and ThenBy(t => t.Title) operations so that the database handles the sorting before the data is materialized, improving query performance.
🤖 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.
Nitpick comments:
In `@WSIST/WSIST.Engine/TestManagement.cs`:
- Around line 301-303: The AsEnumerable() call is materializing the data into
memory before the OrderBy and ThenBy operations, causing sorting to happen in
memory instead of at the database level. Move the AsEnumerable() call to after
the OrderBy(t => t.DueDate) and ThenBy(t => t.Title) operations so that the
database handles the sorting before the data is materialized, improving query
performance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c68c4bca-8cb6-4ee7-a7ec-077c3bb99e5a
📒 Files selected for processing (14)
WSIST/WSIST.Engine/FeedbackManagement.csWSIST/WSIST.Engine/TestManagement.csWSIST/WSIST.UnitTests/UnitTests.csWSIST/WSIST.Web/Components/LanguageToggle.razorWSIST/WSIST.Web/Components/Pages/FeedbackPage.razorWSIST/WSIST.Web/Components/Pages/FeedbackPage.razor.csWSIST/WSIST.Web/Components/Pages/Login.razorWSIST/WSIST.Web/Components/Pages/Settings.razorWSIST/WSIST.Web/Components/Pages/Study.razorWSIST/WSIST.Web/DbRequestCultureProvider.csWSIST/WSIST.Web/Resources/SharedResource.de.resxWSIST/WSIST.Web/Resources/SharedResource.resxWSIST/WSIST.Web/appsettings.jsonWSIST/WSIST.Web/wwwroot/app.css
✅ Files skipped from review due to trivial changes (2)
- WSIST/WSIST.Web/appsettings.json
- WSIST/WSIST.Web/Resources/SharedResource.de.resx
🚧 Files skipped from review as they are similar to previous changes (10)
- WSIST/WSIST.Web/Components/Pages/FeedbackPage.razor
- WSIST/WSIST.Web/Components/LanguageToggle.razor
- WSIST/WSIST.Web/wwwroot/app.css
- WSIST/WSIST.Web/Components/Pages/Settings.razor
- WSIST/WSIST.Web/Components/Pages/FeedbackPage.razor.cs
- WSIST/WSIST.UnitTests/UnitTests.cs
- WSIST/WSIST.Web/DbRequestCultureProvider.cs
- WSIST/WSIST.Web/Resources/SharedResource.resx
- WSIST/WSIST.Web/Components/Pages/Study.razor
- WSIST/WSIST.Web/Components/Pages/Login.razor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Consolidated branch for the remaining WSIST Polish & Feature Batch items (Tasks 4, 5, 3), kept on one branch for review rather than stacked PRs.
Task 4 — Self-service data export (nDSG / GDPR portability)
GET /api/export?format=csv|json,RequireAuthorization, resolves the user from their own auth claims and scopes the query strictly to that user id — one user's export can never contain another's.= + - @etc. prefixed) + UTF-8 BOM for Excel/accented chars. JSON as a second format.Task 5 — In-app feedback / feature requests
Feedbackentity (UserId FK cascade, Message ≤4000, Category/Status enums as ints, CreatedAt indexed) + migration.FeedbackManagementservice (validates/trims, caps length, newest-first admin projection). Authorization stays in the web layer./feedbackpage: any signed-in user submits (category + message); admin-only listing gated byAdmin:Emailconfig (override viaAdmin__Emailenv var; blank = nobody is admin).AuthenticatedComponentBasenow exposesCurrentUserEmail.FeedbackPageto avoid colliding with theFeedbackentity in Razor. 5 new tests.Task 3 — EN/DE language switch
IStringLocalizer<SharedResource>resource (EN + DE satellite). One supported culture (en, so grade/date formatting never changes) but two supported UI cultures (en/de) — only the UI culture flips.User.PreferredLanguagecolumn + migration.DbRequestCultureProviderresolves a signed-in user's stored language ahead of the cookie/Accept-Language providers, so the choice persists across logout/login; null falls through to the browser language (first-login default)./set-language/{culture}writes the culture cookie, persists the preference, and does an open-redirect-safeLocalRedirect.EN / DEtoggle in every nav;<html lang>bound to the current UI culture.landing.jscarries its own EN/DE table keyed off<html lang>), Privacy, Terms, and the error/not-found pages. German is High German (ß spelling, not Swiss); legal sections rendered viaMarkupStringfrom static, developer-authored resource HTML.Verification
dotnet build -c Releaseclean;dotnet test32/32 green;dotnet csharpierclean.WSIST.Web.Resources.SharedResource.resourcesand thede/satellite assembly is emitted.Reviewer notes
Title = "Some Test"modal default is left as-is (changing to empty risks an empty-submit crash — out of scope here).🤖 Generated with Claude Code
Summary by CodeRabbit
/feedbackpage; users can submit categorized messages, and admins can review and update feedback statuses./api/exportsupporting CSV and JSON.