Add content delivery + owner commission accounting

[gamadev1]

Summary

• POST api/v1/downloads resolves a download URL from one of gama-api's three legacy endpoints, selected by ContentType:
  • PastPaper/Test → GET /tests/download/{id}/{type}[/{extraId}] — requires FileType, reports ownerUID/price.paid.
  • Multimedia → GET /files/download/{id} — confirmed live, returns only {url, name}, no owner/price.
  • Exam → GET /exams/download/{id} — confirmed live, same thin shape.
    All confirmed against real gama-api responses (its openapi.yaml doesn't document response bodies).
• Charges the downloader (existing IGameService.SpendPointsAsync, quota-then-points) only when the source reports a price and hasn't already marked it paid — so Multimedia/Exam are unconditionally free through this endpoint, since gama-api reports no price for either.
• Accrues a commission to the content owner (resolved from gama-api's CoreId) only when the source reports an owner and the charge above succeeded — so commission never applies to Multimedia/Exam either. New ContentOwnerCommission ledger, deliberately separate from the points wallet and subscription quota. Commission percent + payout threshold are admin-configurable via ApplicationSettings; points→USD is a fixed first-phase constant (100 points = $1). Payout itself is out of scope for this phase.
• IContentDeliveryProvider (ContentSource-keyed, mirroring the payment-gateway provider pattern) dispatches internally on ContentType to build the right gama-api URL — ContentSource (which external system) and ContentType (which kind of content) are kept as separate axes on purpose.
• CommissionReason is a separate enum from ContentSource, anticipating a future non-download commission trigger (e.g. a blog-publish bonus) that wouldn't involve an external content source at all.
• Collapsed GameService.SpendPointsAsync's PastPaper/Test branch — both route to the same gama-api content (/tests/download), so both now charge FeatureCodes.PastpaperDownload/TransactionType.DownloadPastPaper. The old TestDownload/DownloadTest members stay defined (historical Transaction/quota-consumption data references them) but are no longer written by any code path. Confirmed via the dev DB that no SubscriptionPlanFeature config depended on keeping them separate beyond test fixtures.
• Docs: docs/business/content-delivery.md, plus updates to docs/api/endpoints.md, docs/database/schema.md, docs/database/migrations.md, docs/architecture/design-patterns.md, docs/business/payments-and-points.md, docs/business/subscriptions.md, PROJECT_SNAPSHOT.md.
• Independent of the currently-open legacy-auth-logout PR (Add legacy-auth-bridge logout endpoint #505) — based directly on staging, no overlap.

Test plan

• dotnet build — clean, 0 warnings/errors.
• Manual, live against real gama-api data for all four ContentTypes:
  • PastPaper (paid=true path): no charge, no commission, correct URL.
  • PastPaper with type=extra/extraId (paid=false, price=0 path): charge ran (Transaction row confirmed via DB), commission correctly skipped (owner's CoreId has no linked local account — confirmed via DB).
  • Multimedia: URL resolved, no charge attempted (no price reported).
  • Exam: URL resolved, no charge attempted (no price reported).
  • Missing FileType for PastPaper: clean validation error, not an exception.
• Manual: confirm the two new ApplicationSettings fields are visible/editable via GET/PUT api/v1/admin/applicationsettings as an Admin-role user.
• Manual: exercise commission accrual end-to-end with an owner that does resolve to a local account (both real test cases so far had unlinked owners).

🤖 Generated with Claude Code

[github-actions]

🤖 AI Generated Review

This review was generated automatically and does not replace human review.

Review chunk 1/6

Summary

This pull request introduces a new content delivery system along with accounting for owner commissions. It includes a ContentDeliveryService that handles content downloading from external sources (primarily via the Gama API), calculates necessary charges for users, and processes commissions to content owners. The changes encompass updates to documentation, database migrations, new interfaces, DTOs, and services.

Potential Issues

1. Error Handling: There is limited handling of potential exceptions in external API calls. Consider adding retry logic or more granular error handling, especially for network-related errors.
2. Concurrency: In the commission accrual process, the lack of transaction locking might lead to race conditions if the same resource is accessed concurrently.

Suggestions

1. Security Consideration: Ensure that all JWT tokens used in requests are properly validated and sanitized to prevent injection or misuse.
2. Logging and Monitoring: Enhance the logging mechanism to include more detailed error types and contextual information especially in areas where financial transactions are involved.
3. API Limits: Consider handling API rate limits from the external Gama API to avoid hitting the service's restrictions and ensure continuity.

Positive Feedback

1. Documentation: Comprehensive documentation provided enhances understanding of the new features and maintains an effective knowledge base.
2. Separation of Concerns: The architecture clearly separates different responsibilities into well-defined services and DTOs, promoting maintainability.
3. Use of Enumerations: The use of DownloadContentType and ContentSource enums effectively narrows down allowed types and prevents invalid data from being processed.

Review chunk 2/6

Summary

The pull request introduces content delivery and commission accounting features. It specifically adds classes and enumerations to manage content owner commission data and update related entities like CommissionReason, ContentSource, and ContentType. Additionally, it includes migration scripts to modify the database schema to accommodate the new features.

Potential Issues

• Nullability: ExternalFileType and Owner are nullable; ensure they are handled appropriately in application logic to avoid NullReferenceException.
• Precision and Financial Calculations: The precision for CommissionPercent and AmountUsd should be reviewed to ensure they are suitable for financial calculations.

Suggestions

• Validation: Add validation logic to ensure CommissionPercent and AmountUsd values are within expected ranges and rules defined for financial transactions.
• Security: Consider implementing measures to verify the integrity of commission data, especially those fetched or sent over the network, to prevent tampering.
• Class Documentation Enhancements: The comment on the separation of CommissionReason and ContentSource is useful. Similar documentation for the purpose of nullable properties and edge cases could strengthen understanding.

Positive Feedback

• Clean Separation of Concerns: Different aspects of content management and commission calculation are well-segregated into distinct classes and enumerations, which enhances code modularity.
• Future-proofing: The use of enumeration types for content-related categories allows for future extensions without requiring significant changes to the existing schema.
• Migration Management: Good use of Entity Framework migrations to ensure database schema changes are tracked and applied systematically.

Review chunk 3/6

Summary

The pull request aims to enhance the database schema by defining several entity models related to user identity and educational institutions. These models include user accounts, user identities, roles, claims, schools, and associated entities like images, boards, and comments. The schema changes consist of defining new entities with appropriate properties, constraints, indices, and relationships, representing the application's domain for user and content management.

Potential Issues

1. Security Concerns: Some fields, such as PasswordHash, SecurityStamp, and SensitiveUserDetails, should be adequately secured and managed with encryption procedures if manipulated in the application logic.

2. Database Constraints: Ensure appropriate foreign-key constraints are applied to enforce data integrity, particularly where entity relationships exist (e.g., UserId, RoleId).

3. Potential Index Overhead: While indexes on frequently queried properties can improve performance, consider potential overhead from maintaining too many unique constraints and indices, which can impact write performance.

Suggestions

1. Length Constraints: Consider revisiting the maximum length constraints for various fields (Email, PhoneNumber, etc.) to ensure they align with expected input sizes and any relevant standards.

2. Data Sanitization: Implement mechanisms to sanitize inputs in the application layer, especially for fields such as Keywords, Comment, and Title, to prevent SQL injection or cross-site scripting (XSS) attacks.

3. Enum Mapping: For properties like Gender, ProfileVisibility, ensure there is clear documentation or mapping to possible values if they represent enumerations, aiding maintainability and understanding for future developers.

Positive Feedback

1. Use of Constraints: Proper use of constraints like IsUnique on critical properties, such as Handle and Slug, will effectively maintain data integrity and prevent duplicate entries.

2. Indexing Strategy: Indexes have been correctly applied to improve the performance of common queries, especially for unique and frequently searched fields.

3. Comprehensive Schema Definition: The entity definitions offer a robust schema structure, capturing the relationships and requirements necessary for user and content management within the application domain.

Review chunk 4/6

Summary

This pull request introduces several SQL Server entity configurations for various domain entities in the GamaEdtech application. The changes define properties, foreign key relationships, data types, constraints, and indexes for tables like "SubscriptionPlans", "Tickets", "Tags", and more, enhancing the application's database schema to support content delivery and owner commission accounting.

Potential Issues

No significant issues found.

Suggestions

1. Column Precision: For "SubscriptionPlanPrice" and "VotingPower" entities, ensure that the decimal precision is necessary for the business logic, as higher precision could lead to performance overhead.

2. Unique Constraints and Indexes: Review unique constraints and indexes to ensure they meet functional requirements without duplicating constraints, which can lead to maintenance complexities.

3. Null Constraints: Several entities have Nullable properties such as LastModifyUserId and ExpirationDate. Validate that this aligns with business logic to avoid potential NREs (Null Reference Exceptions) in application code.

Positive Feedback

• Comprehensive use of Fluent API to define schema specifications, leading to maintainable and scalable database configurations.
• Logical organization of foreign key relationships with proper cascading options for deletions.
• Effective use of indexes to enhance query performance, particularly on entity properties likely to be queried frequently, like UserId in several tables.
• Well-structured entity definitions with appropriate data types and constraint definitions, ensuring data integrity and application efficiency.

Review chunk 5/6

Summary

The pull request introduces several key changes, including the addition of content delivery functionality and the implementation of owner commission accounting features. It includes a new migration file to create a ContentOwnerCommissions table, adds a new API Provider for content delivery, and updates relevant controllers and view models to support these new functionalities.

Potential Issues

• Foreign Key Constraints: The new ContentOwnerCommissions table has foreign key constraints without specified OnDelete actions. This could result in unexpected behaviors if related ApplicationUsers are deleted.
• Error Handling: In GamaApiContentDeliveryProvider, ensure that the external API calls handle potential null exceptions or failed responses comprehensively to avoid runtime exceptions.
• Authentication Concerns: The Download method in DownloadsController checks for tokens with a simple string check for null or empty. Using a dedicated method for token validation would be more secure and robust.

Suggestions

• Decoupling Dependency: Consider decoupling the concrete dependency of ILogger<DownloadsController> and IContentDeliveryService by using interfaces if not already, to improve testability and flexibility.
• Enhance Error Messages: When transferring errors, ensure that the messages provide enough context for debugging while not revealing sensitive data.
• Documentation: Ensure that all public methods and classes have XML documentation comments to aid maintainability.

Positive Feedback

• Use of Enums: Good practice using enums like DownloadContentType to manage content types, which enhances code readability and reduces errors.
• Lazy Initialization: Implementing lazy initialization for services such as ILogger and IStringLocalizer is a good practice for performance optimization.
• Comprehensive DTOs: Utilization of well-defined DTOs to transit data between layers, resulting in cleaner and more maintainable code.

Review chunk 6/6

Summary

The pull request introduces a new DownloadContentResponseViewModel class to handle content delivery and owner commission accounting in the application. The class includes properties for content URL, name, whether the content has been paid for, and the source of payment with a specified JSON converter.

Potential Issues

1. Nullable Reference Types: The Url, Name, and PaidBy properties are nullable. Ensure that these null values are correctly handled in the application to prevent potential runtime exceptions.

2. JsonConverter Attribute: The use of JsonConverter with a generic type EnumerationConverter<SpendSource, byte> depends on the implementation of EnumerationConverter. Ensure that this converter correctly handles the serialization and deserialization of the SpendSource values.

Suggestions

1. Validation: Consider adding validation logic or annotations to ensure that essential properties, such as Url and Name, are provided. This can prevent errors related to incomplete data.

2. Documentation: Add XML documentation comments to the class and its properties to improve code readability and maintainability, especially since these properties involve financial transactions.

Positive Feedback

• The use of JsonConverter to handle the serialization of complex types is a good practice. It allows greater flexibility and control over how data is serialized and deserialized.
• The sealed modifier on DownloadContentResponseViewModel is a good choice for performance optimization and indicates that the class is not intended for inheritance.