tercul-backend/internal/app/work/commands.go
google-labs-jules[bot] c2e9a118e2 feat(testing): Increase test coverage and fix authz bugs
This commit significantly increases the test coverage across the application and fixes several underlying bugs that were discovered while writing the new tests.

The key changes include:

- **New Tests:** Added extensive integration and unit tests for GraphQL resolvers, application services, and data repositories, substantially increasing the test coverage for packages like `graphql`, `user`, `translation`, and `analytics`.

- **Authorization Bug Fixes:**
  - Fixed a critical bug where a user creating a `Work` was not correctly associated as its author, causing subsequent permission failures.
  - Corrected the authorization logic in `authz.Service` to properly check for entity ownership by non-admin users.

- **Test Refactoring:**
  - Refactored numerous test suites to use `testify/mock` instead of manual mocks, improving test clarity and maintainability.
  - Isolated integration tests by creating a fresh admin user and token for each test run, eliminating test pollution.
  - Centralized domain errors into `internal/domain/errors.go` and updated repositories to use them, making error handling more consistent.

- **Code Quality Improvements:**
  - Replaced manual mock implementations with `testify/mock` for better consistency.
  - Cleaned up redundant and outdated test files.

These changes stabilize the test suite, improve the overall quality of the codebase, and move the project closer to the goal of 80% test coverage.
2025-10-09 07:03:45 +00:00

375 lines
12 KiB
Go

package work
import (
"context"
"errors"
"fmt"
"tercul/internal/app/analytics"
"tercul/internal/app/authz"
"tercul/internal/domain"
"tercul/internal/domain/search"
platform_auth "tercul/internal/platform/auth"
"tercul/internal/platform/log"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/trace"
"gorm.io/gorm"
)
// WorkCommands contains the command handlers for the work aggregate.
type WorkCommands struct {
repo domain.WorkRepository
authorRepo domain.AuthorRepository
userRepo domain.UserRepository
searchClient search.SearchClient
authzSvc *authz.Service
analyticsSvc analytics.Service
tracer trace.Tracer
}
// NewWorkCommands creates a new WorkCommands handler.
func NewWorkCommands(repo domain.WorkRepository, authorRepo domain.AuthorRepository, userRepo domain.UserRepository, searchClient search.SearchClient, authzSvc *authz.Service, analyticsSvc analytics.Service) *WorkCommands {
return &WorkCommands{
repo: repo,
authorRepo: authorRepo,
userRepo: userRepo,
searchClient: searchClient,
authzSvc: authzSvc,
analyticsSvc: analyticsSvc,
tracer: otel.Tracer("work.commands"),
}
}
// CreateWork creates a new work.
func (c *WorkCommands) CreateWork(ctx context.Context, work *domain.Work) (*domain.Work, error) {
ctx, span := c.tracer.Start(ctx, "CreateWork")
defer span.End()
if work == nil {
return nil, errors.New("work cannot be nil")
}
if work.Title == "" {
return nil, errors.New("work title cannot be empty")
}
if work.Language == "" {
return nil, errors.New("work language cannot be empty")
}
userID, ok := platform_auth.GetUserIDFromContext(ctx)
if !ok {
return nil, domain.ErrUnauthorized
}
user, err := c.userRepo.GetByID(ctx, userID)
if err != nil {
return nil, fmt.Errorf("failed to get user for author creation: %w", err)
}
// Find or create an author for the user
author, err := c.authorRepo.FindByName(ctx, user.Username)
if err != nil {
if errors.Is(err, domain.ErrEntityNotFound) {
// Author doesn't exist, create one
newAuthor := &domain.Author{
Name: user.Username,
}
if err := c.authorRepo.Create(ctx, newAuthor); err != nil {
return nil, fmt.Errorf("failed to create author for user: %w", err)
}
author = newAuthor
} else {
// Another error occurred
return nil, fmt.Errorf("failed to find author: %w", err)
}
}
// Associate the author with the work
work.Authors = []*domain.Author{author}
err = c.repo.Create(ctx, work)
if err != nil {
return nil, err
}
// Index the work in the search client
var content string
if len(work.Translations) > 0 {
content = work.Translations[0].Content
}
if err := c.searchClient.IndexWork(ctx, work, content); err != nil {
// Log the error but don't fail the operation
log.FromContext(ctx).Warn(fmt.Sprintf("Failed to index work after creation: %v", err))
}
return work, nil
}
// UpdateWork updates an existing work after performing an authorization check.
func (c *WorkCommands) UpdateWork(ctx context.Context, work *domain.Work) error {
ctx, span := c.tracer.Start(ctx, "UpdateWork")
defer span.End()
if work == nil {
return fmt.Errorf("%w: work cannot be nil", domain.ErrValidation)
}
if work.ID == 0 {
return fmt.Errorf("%w: work ID cannot be zero", domain.ErrValidation)
}
userID, ok := platform_auth.GetUserIDFromContext(ctx)
if !ok {
return domain.ErrUnauthorized
}
existingWork, err := c.repo.GetByID(ctx, work.ID)
if err != nil {
if errors.Is(err, domain.ErrEntityNotFound) {
return fmt.Errorf("%w: work with id %d not found", domain.ErrEntityNotFound, work.ID)
}
return fmt.Errorf("failed to get work for authorization: %w", err)
}
can, err := c.authzSvc.CanEditWork(ctx, userID, existingWork)
if err != nil {
return err
}
if !can {
return domain.ErrForbidden
}
if work.Title == "" {
return fmt.Errorf("%w: work title cannot be empty", domain.ErrValidation)
}
if work.Language == "" {
return fmt.Errorf("%w: work language cannot be empty", domain.ErrValidation)
}
err = c.repo.Update(ctx, work)
if err != nil {
return err
}
// Index the work in the search client
return c.searchClient.IndexWork(ctx, work, work.Description)
}
// DeleteWork deletes a work by ID after performing an authorization check.
func (c *WorkCommands) DeleteWork(ctx context.Context, id uint) error {
ctx, span := c.tracer.Start(ctx, "DeleteWork")
defer span.End()
if id == 0 {
return fmt.Errorf("%w: invalid work ID", domain.ErrValidation)
}
userID, ok := platform_auth.GetUserIDFromContext(ctx)
if !ok {
return domain.ErrUnauthorized
}
existingWork, err := c.repo.GetByID(ctx, id)
if err != nil {
if errors.Is(err, domain.ErrEntityNotFound) {
return fmt.Errorf("%w: work with id %d not found", domain.ErrEntityNotFound, id)
}
return fmt.Errorf("failed to get work for authorization: %w", err)
}
can, err := c.authzSvc.CanDeleteWork(ctx, userID, existingWork)
if err != nil {
return err
}
if !can {
return domain.ErrForbidden
}
return c.repo.Delete(ctx, id)
}
// AnalyzeWork performs linguistic analysis on a work and its translations.
func (c *WorkCommands) AnalyzeWork(ctx context.Context, workID uint) error {
ctx, span := c.tracer.Start(ctx, "AnalyzeWork")
defer span.End()
logger := log.FromContext(ctx).With("workID", workID)
work, err := c.repo.GetWithTranslations(ctx, workID)
if err != nil {
return fmt.Errorf("failed to get work for analysis: %w", err)
}
logger.Info("Starting analysis for work")
// Analyze the parent work's metadata.
if err := c.analyticsSvc.UpdateWorkReadingTime(ctx, workID); err != nil {
logger.Error(err, "failed to update work reading time")
}
if err := c.analyticsSvc.UpdateWorkComplexity(ctx, workID); err != nil {
logger.Error(err, "failed to update work complexity")
}
if err := c.analyticsSvc.UpdateWorkSentiment(ctx, workID); err != nil {
logger.Error(err, "failed to update work sentiment")
}
// Analyze each translation.
for _, translation := range work.Translations {
logger.Info(fmt.Sprintf("Analyzing translation %d", translation.ID))
if err := c.analyticsSvc.UpdateTranslationReadingTime(ctx, translation.ID); err != nil {
logger.Error(err, fmt.Sprintf("failed to update translation reading time for translation %d", translation.ID))
}
if err := c.analyticsSvc.UpdateTranslationSentiment(ctx, translation.ID); err != nil {
logger.Error(err, fmt.Sprintf("failed to update translation sentiment for translation %d", translation.ID))
}
}
logger.Info("Finished analysis for work")
return nil
}
// MergeWork merges two works, moving all associations from the source to the target and deleting the source.
func (c *WorkCommands) MergeWork(ctx context.Context, sourceID, targetID uint) error {
ctx, span := c.tracer.Start(ctx, "MergeWork")
defer span.End()
if sourceID == targetID {
return fmt.Errorf("%w: source and target work IDs cannot be the same", domain.ErrValidation)
}
userID, ok := platform_auth.GetUserIDFromContext(ctx)
if !ok {
return domain.ErrUnauthorized
}
// The repo is a domain.WorkRepository, which embeds domain.BaseRepository.
// We can use the WithTx method from the base repository to run the merge in a transaction.
err := c.repo.WithTx(ctx, func(tx *gorm.DB) error {
// We need to use the transaction `tx` for all operations inside this function.
// For repository methods that are not on the base repository, we need to
// create a new repository instance that uses the transaction.
// However, since we added `GetWithAssociationsInTx`, we can pass the tx directly.
// Authorization: Ensure user can edit both works
sourceWork, err := c.repo.GetWithAssociationsInTx(ctx, tx, sourceID)
if err != nil {
return fmt.Errorf("failed to get source work: %w", err)
}
targetWork, err := c.repo.GetWithAssociationsInTx(ctx, tx, targetID)
if err != nil {
return fmt.Errorf("failed to get target work: %w", err)
}
canEditSource, err := c.authzSvc.CanEditWork(ctx, userID, sourceWork)
if err != nil {
return err
}
canEditTarget, err := c.authzSvc.CanEditWork(ctx, userID, targetWork)
if err != nil {
return err
}
if !canEditSource || !canEditTarget {
return domain.ErrForbidden
}
// Merge WorkStats
if err = mergeWorkStats(tx, sourceID, targetID); err != nil {
return err
}
// Merge translations intelligently to avoid unique constraint violations.
targetLanguages := make(map[string]bool)
for _, t := range targetWork.Translations {
targetLanguages[t.Language] = true
}
for _, sTranslation := range sourceWork.Translations {
if _, exists := targetLanguages[sTranslation.Language]; !exists {
// No conflict, re-associate this translation with the target work.
if err := tx.Model(&sTranslation).Update("translatable_id", targetID).Error; err != nil {
return fmt.Errorf("failed to merge translation for language %s: %w", sTranslation.Language, err)
}
}
// If a translation for the language already exists on the target, we do nothing.
// The source translation will be implicitly deleted with the source work.
}
// Append many-to-many associations
if err = tx.Model(targetWork).Association("Authors").Append(sourceWork.Authors); err != nil {
return fmt.Errorf("failed to merge authors: %w", err)
}
if err = tx.Model(targetWork).Association("Tags").Append(sourceWork.Tags); err != nil {
return fmt.Errorf("failed to merge tags: %w", err)
}
if err = tx.Model(targetWork).Association("Categories").Append(sourceWork.Categories); err != nil {
return fmt.Errorf("failed to merge categories: %w", err)
}
if err = tx.Model(targetWork).Association("Copyrights").Append(sourceWork.Copyrights); err != nil {
return fmt.Errorf("failed to merge copyrights: %w", err)
}
if err = tx.Model(targetWork).Association("Monetizations").Append(sourceWork.Monetizations); err != nil {
return fmt.Errorf("failed to merge monetizations: %w", err)
}
// Finally, delete the source work.
if err = tx.Select("Authors", "Tags", "Categories", "Copyrights", "Monetizations").Delete(sourceWork).Error; err != nil {
return fmt.Errorf("failed to delete source work associations: %w", err)
}
if err = tx.Delete(&domain.Work{}, sourceID).Error; err != nil {
return fmt.Errorf("failed to delete source work: %w", err)
}
// Re-index the target work in the search client *after* the transaction commits.
// We can't do it here, so we'll do it after the WithTx call.
return nil
})
if err != nil {
return err
}
// Re-index the target work in the search client now that the transaction is committed.
targetWork, err := c.repo.GetByID(ctx, targetID)
if err == nil && targetWork != nil {
if searchErr := c.searchClient.IndexWork(ctx, targetWork, ""); searchErr != nil {
// Log the error but don't fail the main operation
log.FromContext(ctx).Warn(fmt.Sprintf("Failed to re-index target work after merge: %v", searchErr))
}
}
return nil
}
func mergeWorkStats(tx *gorm.DB, sourceWorkID, targetWorkID uint) error {
var sourceStats domain.WorkStats
err := tx.Where("work_id = ?", sourceWorkID).First(&sourceStats).Error
if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
return fmt.Errorf("failed to get source work stats: %w", err)
}
// If source has no stats, there's nothing to do.
if errors.Is(err, gorm.ErrRecordNotFound) {
return nil
}
// Store the original ID to delete later, as the sourceStats.ID might be overwritten.
originalSourceStatsID := sourceStats.ID
var targetStats domain.WorkStats
err = tx.Where("work_id = ?", targetWorkID).First(&targetStats).Error
if errors.Is(err, gorm.ErrRecordNotFound) {
// If target has no stats, create a new stats record for it.
newStats := sourceStats
newStats.ID = 0
newStats.WorkID = targetWorkID
if err = tx.Create(&newStats).Error; err != nil {
return fmt.Errorf("failed to create new target stats: %w", err)
}
} else if err != nil {
return fmt.Errorf("failed to get target work stats: %w", err)
} else {
// Both have stats, so add source to target.
targetStats.Add(&sourceStats)
if err = tx.Save(&targetStats).Error; err != nil {
return fmt.Errorf("failed to save merged target stats: %w", err)
}
}
// Delete the old source stats using the stored original ID.
if err = tx.Delete(&domain.WorkStats{}, originalSourceStatsID).Error; err != nil {
return fmt.Errorf("failed to delete source work stats: %w", err)
}
return nil
}