Repository navigation
Move the remaining controller logic into services; test and tidy the teacher, parent and messages screens - #68
Merged
Merged
Conversation
Not covered before: a payment over several lessons charging their sum, the parent's receipt email and ConfirmationSentAt, the payment detail endpoint, the parent's own payment list, a lesson listed twice being charged once, and an unknown lesson id returning 404 without charging anything. All pass against the current code. Co-Authored-By: Claude <noreply@anthropic.com>
PaymentsController (283 lines) used the DbContext directly. It is now 108 lines of HTTP mapping over: - PaymentQueries: open charges, the "who owes what" summary (the grouping is a named function over typed rows instead of anonymous types), payments, payment detail, and the parent's own views - PaymentService: recording a payment (validation, sum, receipt email, ConfirmationSentAt) and undoing one Responses and messages are unchanged. 303 tests pass. Co-Authored-By: Claude <noreply@anthropic.com>
Not covered before: creating a student with a login (invitation sent, parent linked), a parent id belonging to another teacher being ignored on create, updating details, deleting a student together with their login account, link being idempotent and unlink, and the parent's "my children" and the student's "my profile". All pass against the current code. Co-Authored-By: Claude <noreply@anthropic.com>
StudentsController (261 lines) used the DbContext and UserManager directly, and its detail projection was written twice. It is now 98 lines of HTTP mapping over: - StudentQueries: list, detail (one projection), the parent's children and the student's own profile - StudentService: create (user + student + parent links in one transaction, invitation after commit), update, delete (with the login account), resend invitation, link and unlink a parent Responses and messages are unchanged. 309 tests pass. Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The admin's subject-suggestion endpoints had no tests. Covered: an added name is trimmed and offered to teachers; hiding removes it from what teachers see and adding it again restores it; hiding works for a name not yet added and ignores case; built-in subjects are marked and cannot be hidden; blank names are rejected. All pass against the current code. Co-Authored-By: Claude <noreply@anthropic.com>
…ions AdminController (281 lines) held four unrelated areas. It is now four controllers under Controllers/Admin with the same routes: teachers, subject suggestions, site feedback and the error log. The merge of the built-in catalog, subjects in use and the admin's curated list was written twice, in AdminController and in TeachersController. It now lives once in SubjectSuggestions, used by both; adding and hiding a suggestion share one SetHiddenAsync instead of two copies of the same upsert. Teacher listing and lock/unlock move to TeacherAccounts. 314 tests pass. Co-Authored-By: Claude <noreply@anthropic.com>
…hoto Not covered before: when a profile counts as complete, replacing the subject list (trimmed, blanks and case-insensitive duplicates dropped), adding a duplicate subject and deleting one, availability returned in week order, an invalid window rejecting the whole update and keeping the old schedule, and photo upload type checks and removal. All pass against the current code. Co-Authored-By: Claude <noreply@anthropic.com>
TeachersController (235 lines) held the profile, subjects, photo and availability, all against the DbContext. It is now 70 lines of HTTP mapping over TeacherProfileService (profile, subjects, photo), and availability has its own TeacherAvailabilityController over TeacherAvailabilityService, on the same routes. Window validation is one Parse step instead of being inlined in the loop. Queries on Teacher and TeacherSubject keep their explicit TenantId filter, since those entities are not covered by the global query filter. Responses and messages are unchanged. 322 tests pass. Co-Authored-By: Claude <noreply@anthropic.com>
Every invalid combination of count / end date / indefinite is rejected with its explanatory message and creates no lessons; an end time not after the start and an unknown student are rejected too. None of this was covered. All pass against the current code. Co-Authored-By: Claude <noreply@anthropic.com>
LessonSeriesController packed two 45-line endpoints with validation, local-time derivation, generation, deletion and the family notice. That now lives in LessonSeriesService next to LessonService, split into NewSeries and RemoveFutureOccurrencesAsync; the controller is HTTP mapping only. 330 tests pass. Co-Authored-By: Claude <noreply@anthropic.com>
Neither money view had a test. Income: charged, paid and outstanding for the month only (other months and scheduled lessons excluded, an uncharged lesson counted but not charged), sorted by amount charged; an invalid month is rejected. Payments export: the row carries who paid, the amount, the method and the student. All pass against the current code. Co-Authored-By: Claude <noreply@anthropic.com>
ReportsController repeated the month validation and range in both endpoints and did its grouping inline (methods of ~50 lines). That is now ReportService: one Month helper, typed rows, and SummarizeIncome / SummarizeAttendance / StudentAttendance as small pure functions. ExportController built its three tables inline. ExportService now returns them as ExportTable values; the controller only turns a table into the CSV file response. 333 tests pass. Co-Authored-By: Claude <noreply@anthropic.com>
Not covered before: updating a parent (the email stays), deleting a parent together with their login account, and deleting a parent who has payments being refused while keeping both the parent and their account. All pass against the current code. Co-Authored-By: Claude <noreply@anthropic.com>
Same shape as StudentService: one projection for list and detail (it was written twice), and create, resend invitation, update and delete with their transactions moved out of the controller, which is now HTTP mapping only. 336 tests pass. Co-Authored-By: Claude <noreply@anthropic.com>
The visibility rule (visible to the student implies visible to the parent) was tested for notes created and edited directly, but not for the note written when completing a lesson, which is a separate server path. Also covers rejecting a note linked to another student's lesson. Both pass against the current code. Co-Authored-By: Claude <noreply@anthropic.com>
…rvice "Visible to the student implies visible to the parent" was written out in three places: note create, note update and the note added when a lesson is completed. It is now Note.SetVisibility in the domain, used by all three. NotesController's logic moves to NoteService (one projection instead of two, family access through FamilyMembers); the controller is HTTP mapping only. 338 tests pass. Co-Authored-By: Claude <noreply@anthropic.com>
The anonymous library had no tests. Covered: a teacher who is not public is absent from the list and the city filter, and her card and photo return 404; a public teacher is listed, filterable by subject, city and name, appears in the filter menus, and her card and photo are served. Both pass against the current code. Co-Authored-By: Claude <noreply@anthropic.com>
PublicLibrary holds the anonymous reads, all starting from one PublicTeachers query so the IsPublic restriction is stated once. ContactRequestIntake holds the contact form: the accepting-students check, the explicit TenantId, the notification and the email, split into small builders. PublicController is HTTP mapping only, with its rate limit and cache header unchanged. 340 tests pass. Co-Authored-By: Claude <noreply@anthropic.com>
The screen where the teacher records money received had no tests. Covered: the total owed across parents, opening a parent preselects all their charges, toggling lessons and "all" updates the amount, the exact request sent (selected lessons, yyyy-MM-dd date, blanks as null), nothing is sent with no lesson selected, cancelling a payment only after confirmation, and the "no linked parent" row not opening. Co-Authored-By: Claude <noreply@anthropic.com>
submitChangeRequest (45 lines) inlined the "a new time is required and must end after it starts" check; it is now proposedTimeIsValid. The cancel and reschedule dialogs opened through two copies of the same code; they share openChangeDialog. Tests (8): ?request=1 opens the request form once the children load, a new-lesson request with a blank reason sent as null, a cancel request without a proposed time, a reschedule request rejected without a time or with the end before the start and sent when valid, which requests stay on screen, and no "proposed time" shown after approval. The calendar is stubbed, since FullCalendar is not what is under test. Co-Authored-By: Claude <noreply@anthropic.com>
The weekly availability card had its own state, load and save inside the profile settings component. It is now AvailabilityEditorComponent; the profile component (283 -> 245 lines) keeps the profile, subjects and photo. Tests (9): the editor groups windows by day, adds/removes and saves the whole week, and refuses to save a window that does not end after it starts. The profile screen fills its form and the shared completeness flag, saves blanks as null and rechecks completeness, refuses a negative price, saves subjects trimmed and without blanks, rejects a subject over 100 characters, and rejects a non-image file before uploading. Co-Authored-By: Claude <noreply@anthropic.com>
Profile settings and the first-run setup screen each carried the same photo code: read and reset the file input, reject non-images, crop to a square, upload, remove, and the url/busy/error state. It is now one TeacherPhoto class used by both (the settings screen still shows its "photo updated" toast). Tests (8): TeacherPhoto shows the versioned url, rejects a non-image and resets the input, clears on remove and reports a failed removal. The setup screen requires a subject, accepts a phone or an email, saves the subjects before the profile while keeping the fields it does not edit, skips the profile save if the subjects fail, and "later" goes to the dashboard. Co-Authored-By: Claude <noreply@anthropic.com>
The teacher's inbox and the family's inbox each wrote out the same list updates: mark a thread read when opened, and refresh its row after sending. They now use markRead / withSentMessage / moveToTop in thread-list.util; each inbox keeps its own behavior (the teacher's still moves the thread to the top, the family's does not). Tests (9): the three helpers, and the teacher's inbox: a ?thread= link opens and marks the thread read, the "open" filter hides closed ones, a reply is appended and moves the thread to the top, closing updates the list, recipients read "parent · about student", and a new thread needs a recipient, subject and body, sent trimmed. Co-Authored-By: Claude <noreply@anthropic.com>
yt314
changed the title
This branch was successfully deployed
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.
Summary
This continues #65 and #66. Every controller with real business logic now delegates it to services. The frontend screens that had no tests now have them, and code duplicated between screens lives in one place. Behavior is unchanged.
In every area the tests for the uncovered paths came first, and they passed against the old code before anything moved.
The PR started with payments and students. The later commits on the same branch cover the rest of the controllers and the frontend.
DbContextdirectlyBackend
PaymentQueriesandPaymentService. The controller went from 283 to 99 linesStudentQueriesandStudentService. The detail projection was defined twice and is now defined onceAdminController(281 lines) is split into 4 controllers by area, with the same routes. The subject-suggestion merge was written twice (admin and teachers) and is now oneSubjectSuggestionsservice. Add and hide share one upsert. Teacher listing and lock/unlock moved toTeacherAccountsTeacherProfileServiceandTeacherAvailabilityService, with availability in its own controller. Queries keep their explicitTenantIdfilter, becauseTeacherandTeacherSubjectare not covered by the global filter. The controller went from 235 to 70 linesLessonSeriesService, next toLessonServiceReportServicehas one month helper plus pure summary functions.ExportServicebuilds the tables, and the controller only writes the CSVParentServiceNote.SetVisibilityin the domain. Notes moved toNoteServicePublicLibrary(every read starts from public teachers only) andContactRequestIntakeFrontend
AvailabilityEditorComponentis extractedTeacherPhotoclassTeacherPhotoand the setup screen)thread-list.utilTesting
dotnet test, 340 of 340 pass.ng test, 146 of 146 pass.ng buildis clean.Left as is, on purpose
StudentResources,CalendarEvents,SiteFeedback,ContactRequests,Notifications) are simple CRUD that already has tests. Splitting them wouldn't help.students-listis not split. Its two dialogs are coupled on purpose by focus handling. It got tests in Patch Pkcs advisory; split DbContext, AuthController and student card; more tests #66.Program.csgrew to 313 lines because of the new service registrations. Moving them into one extension method is a natural follow-up.Co-authored with Claude Code.