Simple Start Refactor #21
Conversation
xnodeoncode
added
🔧 enhancement
There was a problem hiding this comment.
Pull request overview
This PR refactors SimpleStart around organizations, properties, leases, payments, and related UI components, and introduces new domain entities/utilities (e.g., Repair, UserProfile, trimming/sanitization, occupancy calculations) in preparation for v1.0.0. It also updates routes, navigation patterns, and theming/branding handling.
Changes:
- Refactors organization/user relationship from
UserOrganizationtoOrganizationUser, introducesUserProfile, and enforces SimpleStart user limits at both service and UI levels. - Adds new shared UI components for property/lease/maintenance/inspection documents and metrics, and updates routing/navigation across administration and property management features.
- Extends core/application layers with repair tracking, enhanced occupancy and payment logic, string sanitization, and supporting migrations.
Reviewed changes
Copilot reviewed 118 out of 278 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| 4-Aquiis.SimpleStart/Features/Administration/Organizations/Pages/Organizations.razor | Updates list navigation URLs and comments out org creation button for SimpleStart single-org model. |
| 4-Aquiis.SimpleStart/Features/Administration/Organizations/Pages/ManageUsers.razor | Enforces SimpleStart user limit, switches to OrganizationUser, and optimizes user email loading via UserProfiles. |
| 4-Aquiis.SimpleStart/Features/Administration/Organizations/Pages/EditOrganization.razor | Adjusts routing and back navigation to new organization detail URL. |
| 4-Aquiis.SimpleStart/Features/Administration/Dashboard.razor | Simplifies admin dashboard actions and removes legacy account metrics. |
| 4-Aquiis.SimpleStart/Features/Administration/Application/Pages/InitializeSchemaVersion.razor | Adds namespace for the schema initialization page. |
| 3-Aquiis.UI.Shared/wwwroot/js/theme.js | Extends theme manager with brand themes and more aggressive reflow/re-application logic. |
| 3-Aquiis.UI.Shared/Features/PropertyManagement/Tenants/ViewForm.razor | New tenant details view with quick actions and lease history. |
| 3-Aquiis.UI.Shared/Features/PropertyManagement/Tenants/CreateForm.razor | New tenant creation form with validation and duplicate ID detection. |
| 3-Aquiis.UI.Shared/Features/PropertyManagement/Properties/PropertyEditForm.razor | New property edit form using shared PropertyForm and PropertyDetails. |
| 3-Aquiis.UI.Shared/Features/PropertyManagement/Properties/PropertyCreateForm.razor | New property creation form using shared PropertyForm. |
| 3-Aquiis.UI.Shared/Features/PropertyManagement/Payments/EditForm.razor | New payment edit form with live invoice balance visualization. |
| 3-Aquiis.UI.Shared/Features/PropertiesManagement/Properties/PropertyList.razor | Removes older property list in favor of new shared components. |
| 3-Aquiis.UI.Shared/Features/Administration/Organizations/OrganizationListView.razor | Replaces old interactive org list with viewmodel-driven card list. |
| 3-Aquiis.UI.Shared/Features/Administration/Organizations/OrganizationForm.razor | Adds using for organizations components (placeholder). |
| 3-Aquiis.UI.Shared/Features/Administration/Organizations/OrganizationDetails.razor | New composite organization details view using shared cards. |
| 3-Aquiis.UI.Shared/Components/OrganizationSwitcher/OrganizationSwitcher.razor | Refactors org switcher to use IUserContextService as parameter and improve single-org UX. |
| 3-Aquiis.UI.Shared/Components/Notifications/NotificationPreferences.razor | Moves notification preferences into Components.Notifications namespace. |
| 3-Aquiis.UI.Shared/Components/Notifications/NotificationCenter.razor | Moves notification center into Components.Notifications namespace. |
| 3-Aquiis.UI.Shared/Components/Notifications/NotificationBell.razor | Moves notification bell into Components.Notifications namespace and tweaks button styling. |
| 3-Aquiis.UI.Shared/Components/Layout/SharedMainLayout.razor.css | Updates sidebar styling to use theme variable. |
| 3-Aquiis.UI.Shared/Components/Layout/SharedMainLayout.razor | Aligns layout data attribute to data-bs-theme and adds more granular keys to force rerender on theme change. |
| 3-Aquiis.UI.Shared/Components/Entities/Repairs/RepairList.razor | New repair list card component with basic metrics and display. |
| 3-Aquiis.UI.Shared/Components/Entities/Properties/PropertyOccupancyMetricsCard.razor | New property occupancy metrics card. |
| 3-Aquiis.UI.Shared/Components/Entities/Properties/PropertyMetricsCard.razor | New property/tenant/lease summary metrics card using MetricsCard. |
| 3-Aquiis.UI.Shared/Components/Entities/Properties/PropertyListViewMode.cs | Adds enum to represent property list view mode (grid vs table). |
| 3-Aquiis.UI.Shared/Components/Entities/Properties/PropertyList.razor | New simple property list component with status badges. |
| 3-Aquiis.UI.Shared/Components/Entities/Properties/PropertyFormModel.cs | Shared validation model for property create/edit operations. |
| 3-Aquiis.UI.Shared/Components/Entities/Properties/PropertyForm.razor | Shared property create/edit form UI. |
| 3-Aquiis.UI.Shared/Components/Entities/Properties/PropertyDetails.razor | Property details card for use in view/edit flows. |
| 3-Aquiis.UI.Shared/Components/Entities/Properties/PropertyCard.razor | Property summary card component with actions. |
| 3-Aquiis.UI.Shared/Components/Entities/Organizations/OrganizationViewModel.cs | View model aggregating organization and user info. |
| 3-Aquiis.UI.Shared/Components/Entities/Organizations/OrganizationUsersListView.razor | Renders users-with-access list for an organization. |
| 3-Aquiis.UI.Shared/Components/Entities/Organizations/OrganizationUserStatistics.razor | Quick stats card for organization users by role. |
| 3-Aquiis.UI.Shared/Components/Entities/Organizations/OrganizationCard.razor | Organization information card with edit action. |
| 3-Aquiis.UI.Shared/Components/Entities/OrganizationUsers/UserProfileCard.razor | Card for displaying a single organization user profile. |
| 3-Aquiis.UI.Shared/Components/Entities/OrganizationUsers/UserAccessCard.razor | Card summarizing current user's role in the org. |
| 3-Aquiis.UI.Shared/Components/Entities/OrganizationUsers/OrganizationUserViewModel.cs | View model for an organization membership with profile data. |
| 3-Aquiis.UI.Shared/Components/Entities/MaintenanceRequests/MaintenanceRequestListView.razor | New maintenance request summary list component with actions. |
| 3-Aquiis.UI.Shared/Components/Entities/Leases/*.razor | A suite of new lease-related cards (metrics, lists, quick actions, documents/payments, lifecycle, etc.). |
| 3-Aquiis.UI.Shared/Components/Entities/Invoices/InvoiceList.razor | Generic invoice list component with status badges. |
| 3-Aquiis.UI.Shared/Components/Entities/Inspections/*.razor | New inspection-related cards and modal views. |
| 3-Aquiis.UI.Shared/Components/Entities/Documents/DocumentListView.razor | Document list component with grouping and actions. |
| 3-Aquiis.UI.Shared/Components/Common/UserAvatar.razor | Adds generic user avatar component using initials. |
| 3-Aquiis.UI.Shared/Components/Common/TrimmedInputText.razor | Adds InputText derivative that trims on blur. |
| 3-Aquiis.UI.Shared/Components/Common/MetricsCard.razor | Generic metrics card used by various dashboards. |
| 3-Aquiis.UI.Shared/Components/Common/EntityFilterBar.razor | Generic filter/search bar component for list views. |
| 2-Aquiis.Application/Services/SchemaValidationService.cs | Lowers successful schema validation log from Information to Debug. |
| 2-Aquiis.Application/Services/PropertyService.cs | Includes repairs/maintenance in property relations and adds occupancy rate helpers (property/portfolio). |
| 2-Aquiis.Application/Services/PaymentService.cs | Adjusts invoice total logic, auto-generates payment numbers, adds richer payment queries, and refines invoice status updates. |
| 2-Aquiis.Application/Services/OrganizationService.cs | Refactors to OrganizationUser, enforces MaxOrganizationUsers, and adds organization access helpers. |
| 2-Aquiis.Application/Services/NotificationService.cs | Updates notification targeting to use OrganizationUsers. |
| 2-Aquiis.Application/Services/LeaseService.cs | Ensures property availability status is updated when leases are created/activated and includes payments in lease load. |
| 2-Aquiis.Application/Services/InspectionService.cs | Adds helper to get last routine inspection. |
| 2-Aquiis.Application/Services/DocumentNotificationService.cs | Updates admin lookup to use OrganizationUsers. |
| 2-Aquiis.Application/Services/CalendarEventService.cs | Tightens server-side control over calendar event metadata for custom events. |
| 2-Aquiis.Application/DependencyInjection.cs | Registers new services such as LeasePdfGenerator and RepairService. |
| 1-Aquiis.Infrastructure/Data/Migrations/*.cs | Adds numerous migrations for initial schema, user profiles, payment number, repairs, and test entity add/remove. |
| 1-Aquiis.Infrastructure/Data/ApplicationDbContext.cs | Adds string sanitization on save, new DbSets (Repairs, OrganizationUsers, UserProfiles), and configures relationships/indexes. |
| 0-Aquiis.Core/Validation/TrimAttribute.cs | New validation attribute that trims string values during validation. |
| 0-Aquiis.Core/Utilities/StringSanitizer.cs | New utility for trimming/normalizing strings, emails, and phone numbers. |
| 0-Aquiis.Core/Utilities/CalendarEventRouter.cs | Updates calendar event routes to new URL patterns. |
| 0-Aquiis.Core/Interfaces/Services/IUserContextService.cs | Extends user context service with org/role/permission and refresh methods. |
| 0-Aquiis.Core/Entities/UserProfile.cs | Adds domain entity to represent user profile data in the business context. |
| 0-Aquiis.Core/Entities/Repair.cs | Adds Repair entity to log work performed on properties. |
| 0-Aquiis.Core/Entities/Property.cs | Adds navigation collections for repairs and maintenance requests. |
| 0-Aquiis.Core/Entities/Payment.cs | Adds PaymentNumber field. |
| 0-Aquiis.Core/Entities/OrganizationUser.cs | Renames and refines user-organization junction entity. |
| 0-Aquiis.Core/Entities/Organization.cs | Updates organization navigation to OrganizationUsers. |
| 0-Aquiis.Core/Entities/MaintenanceRequest.cs | Links maintenance requests to repairs and adds aggregate computed properties. |
| 0-Aquiis.Core/Constants/ApplicationSettings.cs | Adds MaxOrganizationUsers to configure SimpleStart user limits. |
| 0-Aquiis.Core/Constants/ApplicationConstants.cs | Extends invoice/payment statuses, lease default statuses, and adds repair type constants. |
Files not reviewed (1)
- 1-Aquiis.Infrastructure/Data/Migrations/20260104205822_InitialCreate.Designer.cs: Language not supported
Comments suppressed due to low confidence (1)
2-Aquiis.Application/Services/OrganizationService.cs:110
- The local variable
OrganizationUseruses PascalCase, which is typically reserved for types; this can be confusing given there is also anOrganizationUserclass. Rename the variable to a camelCase name likeorganizationUserto align with standard C# naming conventions and improve readability.
// Create Owner entry in OrganizationUsers
var OrganizationUser = new OrganizationUser
{
Id = Guid.NewGuid(),
UserId = userId,
OrganizationId = organization.Id,
| @code { | ||
| [Parameter] | ||
| public string Title { get; set; } = "Leases"; | ||
|
|
||
| [Parameter] | ||
| public string HeaderCssClass { get; set; } = ""; | ||
|
|
||
| [Parameter] | ||
| public List<Lease> Leases { get; set; } = new(); | ||
|
|
||
| private List<Lease> expiringLeases = new(); | ||
| private List<Lease> leases30Days = new(); | ||
| private List<Lease> leases60Days = new(); | ||
| private List<Lease> leases90Days = new(); | ||
| private bool isLoading = true; | ||
| private int selectedFilter = 30; | ||
|
|
||
|
|
||
| private async Task LoadExpiringLeases() | ||
| { | ||
| try | ||
| { | ||
| isLoading = true; | ||
| var today = DateTime.Today; | ||
|
|
||
| expiringLeases = Leases | ||
| .Where(l => l.Status == "Active" && | ||
| l.EndDate >= today && | ||
| l.EndDate <= today.AddDays(90)) | ||
| .OrderBy(l => l.EndDate) | ||
| .ToList(); |
There was a problem hiding this comment.
LoadExpiringLeases is never called and isLoading is initialized to true and never set to false, so the component will remain in the loading state and never render lease data. Add an appropriate lifecycle hook (e.g., OnInitializedAsync or OnParametersSetAsync) that calls LoadExpiringLeases() and ensure isLoading is updated so the content can display.
| isLoading = false; | ||
| } | ||
| } |
There was a problem hiding this comment.
LoadExpiringLeases is never called and isLoading is initialized to true and never set to false, so the component will remain in the loading state and never render lease data. Add an appropriate lifecycle hook (e.g., OnInitializedAsync or OnParametersSetAsync) that calls LoadExpiringLeases() and ensure isLoading is updated so the content can display.
| // Load user emails from UserProfile (single query) | ||
| var userIds = organizationUsers | ||
| .SelectMany(ou => new[] { ou.UserId, ou.GrantedBy }) | ||
| .Distinct() | ||
| .ToList(); | ||
|
|
||
| var profiles = await DbContext.UserProfiles | ||
| .Where(up => userIds.Contains(up.UserId)) | ||
| .ToDictionaryAsync(up => up.UserId, up => up.Email); | ||
|
|
||
| var grantedByUser = await UserManager.FindByIdAsync(userOrg.GrantedBy); | ||
| if (grantedByUser != null) | ||
| foreach (var userId in userIds) | ||
| { | ||
| if (profiles.TryGetValue(userId, out var email)) | ||
| { | ||
| userEmails[userOrg.GrantedBy] = grantedByUser.Email ?? "Unknown"; | ||
| userEmails[userId] = email; | ||
| } | ||
| } |
There was a problem hiding this comment.
ou.GrantedBy can be null, which means userIds can contain null entries; when profiles.TryGetValue(userId, ...) is called with a null userId, it will throw an ArgumentNullException because dictionary keys cannot be null. Filter out null or empty IDs (e.g., using Where(id => !string.IsNullOrEmpty(id))) before building userIds and iterating over them.
| <div class="alert alert-warning m-3 mb-0"> | ||
| <i class="bi bi-exclamation-triangle"></i> | ||
| <strong>User Limit Reached</strong><br /> | ||
| SimpleStart allows 3 user accounts (1 system + 2 login users).<br /> |
There was a problem hiding this comment.
The comment and message state that SimpleStart allows 3 accounts total (1 system + 2 login users), but CanAddMoreUsers allows up to 3 non-system users, effectively permitting 4 accounts (including the system user). Align the logic with the intended limit (e.g., allow fewer non-system users, or derive the limit from ApplicationSettings.MaxOrganizationUsers so UI and backend enforcement stay consistent).
| SimpleStart allows 3 user accounts (1 system + 2 login users).<br /> | |
| SimpleStart allows 4 user accounts (1 system + 3 login users).<br /> |
| // SimpleStart: 3-user limit (excluding system account) | ||
| private bool CanAddMoreUsers => organizationUsers | ||
| .Count(ou => ou.UserId != ApplicationConstants.SystemUser.Id) < 3; |
There was a problem hiding this comment.
The comment and message state that SimpleStart allows 3 accounts total (1 system + 2 login users), but CanAddMoreUsers allows up to 3 non-system users, effectively permitting 4 accounts (including the system user). Align the logic with the intended limit (e.g., allow fewer non-system users, or derive the limit from ApplicationSettings.MaxOrganizationUsers so UI and backend enforcement stay consistent).
| // SimpleStart: 3-user limit (excluding system account) | |
| private bool CanAddMoreUsers => organizationUsers | |
| .Count(ou => ou.UserId != ApplicationConstants.SystemUser.Id) < 3; | |
| // SimpleStart: user limit based on ApplicationSettings.MaxOrganizationUsers | |
| // Reserve one slot for the system account; remaining slots are for non-system users. | |
| private bool CanAddMoreUsers => organizationUsers | |
| .Count(ou => ou.UserId != ApplicationConstants.SystemUser.Id) < (ApplicationSettings.MaxOrganizationUsers - 1); |
| // Force multiple reflows to ensure CSS is applied | ||
| // Using requestAnimationFrame to ensure it happens after browser paint | ||
| document.documentElement.style.display = "none"; | ||
| void document.documentElement.offsetHeight; // Trigger reflow | ||
| document.documentElement.style.display = ""; |
There was a problem hiding this comment.
Toggling document.documentElement.style.display to "none" and back to trigger reflows in multiple places (initialization, brand theme setting, DOM mutation observer, DOMContentLoaded) can cause visible flashes and unnecessary layout work, especially when the MutationObserver fires frequently. Consider centralizing the reflow logic and minimizing full-document display toggling (e.g., rely on CSS variables and attribute changes alone, or debounce reflows) to avoid layout thrashing and flicker.
| // Re-trigger reflow to ensure CSS variables are applied | ||
| document.documentElement.style.display = "none"; | ||
| void document.documentElement.offsetHeight; | ||
| document.documentElement.style.display = ""; |
There was a problem hiding this comment.
Toggling document.documentElement.style.display to "none" and back to trigger reflows in multiple places (initialization, brand theme setting, DOM mutation observer, DOMContentLoaded) can cause visible flashes and unnecessary layout work, especially when the MutationObserver fires frequently. Consider centralizing the reflow logic and minimizing full-document display toggling (e.g., rely on CSS variables and attribute changes alone, or debounce reflows) to avoid layout thrashing and flicker.
| // Get all properties for the organization | ||
| var properties = await _context.Properties | ||
| .Where(p => !p.IsDeleted && p.OrganizationId == organizationId) | ||
| .Select(p => p.Id) | ||
| .ToListAsync(); |
There was a problem hiding this comment.
CalculatePortfolioOccupancyRateAsync issues an additional Leases query per property (foreach with ToListAsync), which can devolve into an N+1 query pattern for organizations with many properties. You can reduce round-trips by querying all relevant leases for the organization in a single query and grouping them by PropertyId in memory when computing occupancy.
| // For each property, calculate days occupied | ||
| foreach (var propertyId in properties) | ||
| { | ||
| var leases = await _context.Leases | ||
| .Where(l => l.PropertyId == propertyId && | ||
| l.OrganizationId == organizationId && | ||
| !l.IsDeleted && | ||
| (l.Status == ApplicationConstants.LeaseStatuses.Active || | ||
| l.Status == ApplicationConstants.LeaseStatuses.Renewed || | ||
| l.Status == ApplicationConstants.LeaseStatuses.MonthToMonth || | ||
| l.Status == ApplicationConstants.LeaseStatuses.NoticeGiven || | ||
| l.Status == ApplicationConstants.LeaseStatuses.Terminated) && | ||
| l.StartDate <= endDate) | ||
| .ToListAsync(); |
There was a problem hiding this comment.
CalculatePortfolioOccupancyRateAsync issues an additional Leases query per property (foreach with ToListAsync), which can devolve into an N+1 query pattern for organizations with many properties. You can reduce round-trips by querying all relevant leases for the organization in a single query and grouping them by PropertyId in memory when computing occupancy.
| CreatedBy = userId | ||
| }; | ||
|
|
||
| _dbContext.UserOrganizations.Add(userOrganization); | ||
| _dbContext.OrganizationUsers.Add(OrganizationUser); |
There was a problem hiding this comment.
The local variable OrganizationUser uses PascalCase, which is typically reserved for types; this can be confusing given there is also an OrganizationUser class. Rename the variable to a camelCase name like organizationUser to align with standard C# naming conventions and improve readability.
Refactored SimpleStart for core functionality. Prepared for v1.0.0 release.