From 5d6f378fce11f38d7d21cc8aa29826e68d6db059 Mon Sep 17 00:00:00 2001 From: Florian Sylvain Date: Wed, 28 Oct 2026 17:54:10 +0100 Subject: [PATCH] refac: stale test entries, failing web tests, Postgre backup bug, Hardcoded MySQL table count, Stale Postgre connection in test, own-scope visibility filter, GetAll ignoring its error, JSON tags in use-case types, 31 copies of DESKTOP_QUERY, ProfileView endpoint paths --- api/handlers/auth/loginHandler.go | 7 +- api/handlers/auth/meHandler.go | 11 +- api/handlers/ownership/ownershipHandler.go | 3 +- api/handlers/role/roleHandler.go | 11 +- api/handlers/user/inviteHandler.go | 49 ++++---- api/handlers/user/shared.go | 5 +- api/pkg/payload/role.go | 91 ++++++++++++++ .../article/articleRepositoryInterface.go | 20 +-- .../domain/entry/entryRepositoryInterface.go | 1 + .../invite/inviteRepositoryInterface.go | 2 +- .../domain/user/userRepositoryInterface.go | 2 +- .../persistence/database/postgresBackup.go | 4 +- .../repositories/articleRepository.go | 20 ++- .../repositories/entryRepository.go | 6 +- .../repositories/inviteRepository.go | 8 +- .../repositories/userRepository.go | 8 +- .../infrastructure/useCases/AcceptInvite.go | 18 ++- .../infrastructure/useCases/CreateInvite.go | 24 +++- .../infrastructure/useCases/CreateUser.go | 23 ++-- .../useCases/EnforceRetention.go | 12 +- internal/infrastructure/useCases/GetInvite.go | 6 +- .../infrastructure/useCases/ListArticles.go | 119 +++++------------- .../infrastructure/useCases/ListEntries.go | 9 +- internal/infrastructure/useCases/ListUsers.go | 2 +- .../useCases/ListUsersAndInvites.go | 15 ++- .../useCases/TransferOwnership.go | 14 +-- .../infrastructure/useCases/inviteLookup.go | 6 +- internal/infrastructure/useCases/roleRef.go | 8 +- internal/infrastructure/useCases/roleView.go | 10 +- test/accept_invitation_atomic_test.go | 6 +- test/api_catalogue_drift_test.go | 24 ---- test/article_test.go | 23 +++- test/custom_content_types_support_test.go | 2 +- test/get_all_error_propagation_test.go | 74 +++++++++++ test/invitations_test.go | 7 +- test/mysql_server_test.go | 7 +- test/own_scope_visibility_persistence_test.go | 111 ++++++++++++++++ test/personal_data_retention_test.go | 2 +- test/postgres_server_test.go | 7 +- .../registration_status_endpoint_http_test.go | 2 +- test/repository_contract_test.go | 13 +- test/site_cleanup_test.go | 12 +- ...nverified_users_and_reregistration_test.go | 2 +- test/use_case_baseline_test.go | 2 +- test/user_test.go | 7 +- test/users_api_hardening_test.go | 2 +- test/users_api_persistence_test.go | 2 +- web/src/components/ApiSearchPalette.vue | 6 +- web/src/components/AppPaginator.vue | 6 +- web/src/components/AuthorList.vue | 5 +- web/src/components/ColumnPicker.vue | 5 +- web/src/components/ContentToolbar.vue | 6 +- web/src/components/DirectoryList.vue | 5 +- web/src/components/DirectoryPanel.vue | 5 +- web/src/components/MediaBulkBar.vue | 6 +- web/src/components/MediaDetailPanel.vue | 5 +- web/src/components/MediaFieldControl.vue | 5 +- web/src/components/MediaPicker.vue | 5 +- web/src/components/MediaToolbar.vue | 5 +- web/src/components/MediaUsageTooltip.vue | 6 +- web/src/components/SchemaBuilderPage.vue | 6 +- web/src/components/SeoPanel.vue | 6 +- web/src/components/UserMenu.vue | 5 +- web/src/components/YourRoleCard.vue | 6 +- .../variants/VariantFormSlideover.vue | 5 +- web/src/composables/__tests__/useApi.spec.ts | 2 +- web/src/composables/useContentListPage.ts | 5 +- web/src/composables/useEntryEditorPage.ts | 5 +- web/src/composables/useIsDesktop.ts | 7 ++ web/src/composables/useProfile.ts | 25 ++++ web/src/i18n/spa.spec.ts | 11 +- web/src/views/ActivityView.vue | 6 +- web/src/views/ApiKeysView.vue | 5 +- web/src/views/AuthorsView.vue | 5 +- web/src/views/CachePage.vue | 5 +- web/src/views/MediaView.vue | 6 +- web/src/views/ProfileView.vue | 17 +-- web/src/views/RoleEditorView.vue | 5 +- web/src/views/RolesListView.vue | 6 +- web/src/views/SchemaView.vue | 5 +- web/src/views/SitesView.vue | 5 +- web/src/views/UsersView.vue | 5 +- web/src/views/VariantsPage.vue | 5 +- .../__tests__/EntryEditorRelation.spec.ts | 4 +- .../EntryEditorRelationStates.spec.ts | 6 +- .../views/__tests__/IntegrationView.spec.ts | 10 +- .../views/__tests__/SeoIntegration.spec.ts | 2 +- 87 files changed, 696 insertions(+), 373 deletions(-) create mode 100644 api/pkg/payload/role.go create mode 100644 test/get_all_error_propagation_test.go create mode 100644 test/own_scope_visibility_persistence_test.go create mode 100644 web/src/composables/useIsDesktop.ts create mode 100644 web/src/composables/useProfile.ts diff --git a/api/handlers/auth/loginHandler.go b/api/handlers/auth/loginHandler.go index bd41f12..5703663 100644 --- a/api/handlers/auth/loginHandler.go +++ b/api/handlers/auth/loginHandler.go @@ -139,7 +139,12 @@ func (h *LoginHandler) Register(w http.ResponseWriter, r *http.Request) { } func (h *LoginHandler) RegistrationStatus(w http.ResponseWriter, _ *http.Request) { - body, _ := json.Marshal(map[string]bool{"open": h.CreateUseCase.IsRegistrationOpen()}) + open, err := h.CreateUseCase.IsRegistrationOpen() + if err != nil { + http.Error(w, apperr.RegistrationFailedErrorMessage, http.StatusInternalServerError) + return + } + body, _ := json.Marshal(map[string]bool{"open": open}) w.Header().Set("Cache-Control", "no-store") _, _ = w.Write(body) } diff --git a/api/handlers/auth/meHandler.go b/api/handlers/auth/meHandler.go index 7e6bb06..81f82b9 100644 --- a/api/handlers/auth/meHandler.go +++ b/api/handlers/auth/meHandler.go @@ -3,6 +3,7 @@ package auth import ( "RenewCMS/api/pkg/apperr" "RenewCMS/api/pkg/claims" + "RenewCMS/api/pkg/payload" "RenewCMS/api/pkg/publicuser" "RenewCMS/internal/domain/author" "RenewCMS/internal/domain/role" @@ -33,9 +34,9 @@ type meAuthor struct { } type meAccess struct { - Role useCases.RoleRef `json:"role"` - Permissions []role.Grant `json:"permissions"` - RoleAssignedBy *useCases.RoleAssigner `json:"role_assigned_by"` + Role payload.RoleRef `json:"role"` + Permissions []role.Grant `json:"permissions"` + RoleAssignedBy *payload.RoleAssigner `json:"role_assigned_by"` } type meResponse struct { @@ -46,9 +47,9 @@ type meResponse struct { func toMeAccess(access useCases.Access) meAccess { return meAccess{ - Role: useCases.NewRoleRef(access.Role), + Role: payload.RoleRefOf(useCases.NewRoleRef(access.Role)), Permissions: access.Permissions, - RoleAssignedBy: access.RoleAssignedBy, + RoleAssignedBy: payload.RoleAssignerPtrOf(access.RoleAssignedBy), } } diff --git a/api/handlers/ownership/ownershipHandler.go b/api/handlers/ownership/ownershipHandler.go index 8e5e6db..5ef0d3f 100644 --- a/api/handlers/ownership/ownershipHandler.go +++ b/api/handlers/ownership/ownershipHandler.go @@ -3,6 +3,7 @@ package ownership import ( "RenewCMS/api/pkg/access" "RenewCMS/api/pkg/apperr" + "RenewCMS/api/pkg/payload" validator "RenewCMS/api/pkg/validator" "RenewCMS/internal/domain/user" "RenewCMS/internal/infrastructure/useCases" @@ -45,7 +46,7 @@ func (h *Handler) Transfer(w http.ResponseWriter, r *http.Request) { return } - body, err := json.Marshal(transferred) + body, err := json.Marshal(payload.OwnershipTransferOf(transferred)) if err != nil { http.Error(w, "Internal server error", http.StatusInternalServerError) return diff --git a/api/handlers/role/roleHandler.go b/api/handlers/role/roleHandler.go index 524ff53..0e25caa 100644 --- a/api/handlers/role/roleHandler.go +++ b/api/handlers/role/roleHandler.go @@ -3,6 +3,7 @@ package role import ( "RenewCMS/api/pkg/access" "RenewCMS/api/pkg/apperr" + "RenewCMS/api/pkg/payload" validator "RenewCMS/api/pkg/validator" domain "RenewCMS/internal/domain/role" "RenewCMS/internal/infrastructure/useCases" @@ -152,7 +153,7 @@ func (h *Handler) List(w http.ResponseWriter, _ *http.Request) { writeRoleError(w, err) return } - writeJSON(w, http.StatusOK, listed) + writeJSON(w, http.StatusOK, payload.RoleViewsOf(listed)) } func (h *Handler) Get(w http.ResponseWriter, r *http.Request) { @@ -165,7 +166,7 @@ func (h *Handler) Get(w http.ResponseWriter, r *http.Request) { writeRoleError(w, err) return } - writeJSON(w, http.StatusOK, found) + writeJSON(w, http.StatusOK, payload.RoleViewOf(found)) } func (h *Handler) Create(w http.ResponseWriter, r *http.Request) { @@ -180,7 +181,7 @@ func (h *Handler) Create(w http.ResponseWriter, r *http.Request) { writeRoleError(w, err) return } - writeJSON(w, http.StatusCreated, created) + writeJSON(w, http.StatusCreated, payload.RoleViewOf(created)) } func (h *Handler) Update(w http.ResponseWriter, r *http.Request) { @@ -199,7 +200,7 @@ func (h *Handler) Update(w http.ResponseWriter, r *http.Request) { writeRoleError(w, err) return } - writeJSON(w, http.StatusOK, updated) + writeJSON(w, http.StatusOK, payload.RoleViewOf(updated)) } func (h *Handler) Reset(w http.ResponseWriter, r *http.Request) { @@ -216,7 +217,7 @@ func (h *Handler) Reset(w http.ResponseWriter, r *http.Request) { writeRoleError(w, err) return } - writeJSON(w, http.StatusOK, restored) + writeJSON(w, http.StatusOK, payload.RoleViewOf(restored)) } func (h *Handler) Delete(w http.ResponseWriter, r *http.Request) { diff --git a/api/handlers/user/inviteHandler.go b/api/handlers/user/inviteHandler.go index 2f47ae6..0031b57 100644 --- a/api/handlers/user/inviteHandler.go +++ b/api/handlers/user/inviteHandler.go @@ -4,6 +4,7 @@ import ( "RenewCMS/api/pkg/access" "RenewCMS/api/pkg/apperr" "RenewCMS/api/pkg/claims" + "RenewCMS/api/pkg/payload" validator "RenewCMS/api/pkg/validator" "RenewCMS/internal/domain/invite" "RenewCMS/internal/domain/user" @@ -30,34 +31,34 @@ type CreateInviteRequest struct { } type inviteResponse struct { - ID uint32 `json:"id"` - Kind string `json:"kind"` - Email string `json:"email"` - Status string `json:"status"` - Role useCases.RoleRef `json:"role"` - CreatedAt time.Time `json:"created_at"` - ExpiresAt time.Time `json:"expires_at"` + ID uint32 `json:"id"` + Kind string `json:"kind"` + Email string `json:"email"` + Status string `json:"status"` + Role payload.RoleRef `json:"role"` + CreatedAt time.Time `json:"created_at"` + ExpiresAt time.Time `json:"expires_at"` } type inviteListItem struct { - ID uint32 `json:"id"` - Kind string `json:"kind"` - Email string `json:"email"` - Role *useCases.RoleRef `json:"role"` - Status string `json:"status"` - CreatedAt time.Time `json:"created_at"` + ID uint32 `json:"id"` + Kind string `json:"kind"` + Email string `json:"email"` + Role *payload.RoleRef `json:"role"` + Status string `json:"status"` + CreatedAt time.Time `json:"created_at"` } type userListItem struct { - ID uint32 `json:"id"` - Kind string `json:"kind"` - Username string `json:"username"` - DisplayName string `json:"display_name"` - Email string `json:"email"` - Role *useCases.RoleRef `json:"role"` - RoleAssignedBy *useCases.RoleAssigner `json:"role_assigned_by"` - Status string `json:"status"` - CreatedAt time.Time `json:"created_at"` + ID uint32 `json:"id"` + Kind string `json:"kind"` + Username string `json:"username"` + DisplayName string `json:"display_name"` + Email string `json:"email"` + Role *payload.RoleRef `json:"role"` + RoleAssignedBy *payload.RoleAssigner `json:"role_assigned_by"` + Status string `json:"status"` + CreatedAt time.Time `json:"created_at"` } func NewInviteHandler(createInvite *useCases.CreateInviteUseCase, cancelInvite *useCases.CancelInviteUseCase, renewInvite *useCases.RenewInviteUseCase, listUsersAndInvites *useCases.ListUsersAndInvitesUseCase) *InviteHandler { @@ -107,7 +108,7 @@ func writeInviteResponse(w http.ResponseWriter, status int, entry useCases.Invit Kind: useCases.EntryKindInvite, Email: entry.Email, Status: string(entry.StatusAt(time.Now())), - Role: entry.Role, + Role: payload.RoleRefOf(entry.Role), CreatedAt: entry.CreatedAt, ExpiresAt: entry.ExpiresAt, }) @@ -212,7 +213,7 @@ func (h *InviteHandler) ListUsers(w http.ResponseWriter, _ *http.Request) { ID: entry.ID, Kind: entry.Kind, Email: entry.Email, - Role: entry.Role, + Role: payload.RoleRefPtrOf(entry.Role), Status: entry.Status, CreatedAt: entry.CreatedAt, }) diff --git a/api/handlers/user/shared.go b/api/handlers/user/shared.go index e4c690b..003cd79 100644 --- a/api/handlers/user/shared.go +++ b/api/handlers/user/shared.go @@ -2,6 +2,7 @@ package user import ( "RenewCMS/api/pkg/apperr" + "RenewCMS/api/pkg/payload" "RenewCMS/internal/domain/role" "RenewCMS/internal/infrastructure/useCases" "encoding/json" @@ -16,8 +17,8 @@ func userListItemOf(entry useCases.DirectoryEntry) userListItem { Username: entry.Username, DisplayName: entry.DisplayName, Email: entry.Email, - Role: entry.Role, - RoleAssignedBy: entry.RoleAssignedBy, + Role: payload.RoleRefPtrOf(entry.Role), + RoleAssignedBy: payload.RoleAssignerPtrOf(entry.RoleAssignedBy), Status: entry.Status, CreatedAt: entry.CreatedAt, } diff --git a/api/pkg/payload/role.go b/api/pkg/payload/role.go new file mode 100644 index 0000000..b9c81af --- /dev/null +++ b/api/pkg/payload/role.go @@ -0,0 +1,91 @@ +package payload + +import ( + "RenewCMS/internal/domain/role" + "RenewCMS/internal/infrastructure/useCases" + "time" +) + +type RoleRef struct { + ID uint32 `json:"id"` + Name string `json:"name"` + BuiltIn bool `json:"built_in"` + Locked bool `json:"locked"` +} + +type RoleAssigner struct { + ID uint32 `json:"id"` + Name string `json:"name"` +} + +type RoleEditor struct { + ID uint32 `json:"id"` + Name string `json:"name"` +} + +type RoleView struct { + role.Role + Members int `json:"members"` + UpdatedAt *time.Time `json:"updated_at"` + UpdatedBy *RoleEditor `json:"updated_by"` +} + +type OwnershipSuccessor struct { + ID uint32 `json:"id"` + Name string `json:"name"` +} + +type OwnershipPredecessor struct { + ID uint32 `json:"id"` + Name string `json:"name"` + Role RoleRef `json:"role"` +} + +type OwnershipTransfer struct { + Owner OwnershipSuccessor `json:"owner"` + FormerOwner OwnershipPredecessor `json:"former_owner"` +} + +func RoleRefOf(ref useCases.RoleRef) RoleRef { + return RoleRef(ref) +} + +func RoleRefPtrOf(ref *useCases.RoleRef) *RoleRef { + if ref == nil { + return nil + } + converted := RoleRefOf(*ref) + return &converted +} + +func RoleAssignerPtrOf(assigner *useCases.RoleAssigner) *RoleAssigner { + if assigner == nil { + return nil + } + converted := RoleAssigner(*assigner) + return &converted +} + +func RoleViewOf(view useCases.RoleView) RoleView { + converted := RoleView{Role: view.Role, Members: view.Members, UpdatedAt: view.UpdatedAt} + if view.UpdatedBy != nil { + editor := RoleEditor(*view.UpdatedBy) + converted.UpdatedBy = &editor + } + return converted +} + +func RoleViewsOf(views []useCases.RoleView) []RoleView { + converted := make([]RoleView, 0, len(views)) + for _, view := range views { + converted = append(converted, RoleViewOf(view)) + } + return converted +} + +func OwnershipTransferOf(transfer useCases.OwnershipTransfer) OwnershipTransfer { + return OwnershipTransfer{ + Owner: OwnershipSuccessor(transfer.Owner), + FormerOwner: OwnershipPredecessor{ID: transfer.FormerOwner.ID, Name: transfer.FormerOwner.Name, Role: RoleRefOf(transfer.FormerOwner.Role)}, + } +} diff --git a/internal/domain/article/articleRepositoryInterface.go b/internal/domain/article/articleRepositoryInterface.go index dd913ae..84a8e37 100644 --- a/internal/domain/article/articleRepositoryInterface.go +++ b/internal/domain/article/articleRepositoryInterface.go @@ -30,14 +30,15 @@ func DefaultSort() Sort { } type Filters struct { - IsOnline *bool - SiteID *uint32 - Page int - Size int - Q string - Status string - Sort Sort - Unpaged bool + IsOnline *bool + SiteID *uint32 + VisibleTo *uint32 + Page int + Size int + Q string + Status string + Sort Sort + Unpaged bool } type Nullable[T any] struct { @@ -89,9 +90,10 @@ type Repository interface { FindByFilters(filters Filters) ([]Article, error) FindPage(filters Filters) (PageResult, error) CountByVisibility(onlineOnly bool) (Counts, error) + CountVisibleTo(userID uint32) (Counts, error) Get(id uint32) (Article, error) GetByName(name string) (Article, error) - GetAll() []Article + GetAll() ([]Article, error) Create(post Article) (Article, error) UpdateBody(id uint32, body string) (Article, error) Update(id uint32, update Update) (Article, error) diff --git a/internal/domain/entry/entryRepositoryInterface.go b/internal/domain/entry/entryRepositoryInterface.go index 2c56797..96e7630 100644 --- a/internal/domain/entry/entryRepositoryInterface.go +++ b/internal/domain/entry/entryRepositoryInterface.go @@ -48,6 +48,7 @@ func DefaultSort() Sort { type Filters struct { ContentTypeID uint32 + CreatedBy *uint32 Page int Size int Sort Sort diff --git a/internal/domain/invite/inviteRepositoryInterface.go b/internal/domain/invite/inviteRepositoryInterface.go index e5e0713..a53ac76 100644 --- a/internal/domain/invite/inviteRepositoryInterface.go +++ b/internal/domain/invite/inviteRepositoryInterface.go @@ -7,7 +7,7 @@ import ( type Repository interface { Create(invite Invite) (Invite, error) - GetAll() []Invite + GetAll() ([]Invite, error) Delete(id uint32) error Renew(id uint32, hashedCode string, expiresAt time.Time, createdAt time.Time) (Invite, error) AcceptWithUser(inviteID uint32, acceptedAt time.Time, newUser user.User) (user.User, error) diff --git a/internal/domain/user/userRepositoryInterface.go b/internal/domain/user/userRepositoryInterface.go index fbc04c5..45bc270 100644 --- a/internal/domain/user/userRepositoryInterface.go +++ b/internal/domain/user/userRepositoryInterface.go @@ -6,7 +6,7 @@ type Repository interface { Get(id uint32) (User, error) GetByUsername(username string) (User, error) GetByEmail(email string) (User, error) - GetAll() []User + GetAll() ([]User, error) FindByIDs(ids []uint32) ([]User, error) Create(user User) (User, error) Delete(id uint32) error diff --git a/internal/infrastructure/persistence/database/postgresBackup.go b/internal/infrastructure/persistence/database/postgresBackup.go index 8a1a4e7..b74e196 100644 --- a/internal/infrastructure/persistence/database/postgresBackup.go +++ b/internal/infrastructure/persistence/database/postgresBackup.go @@ -48,6 +48,8 @@ var postgresNumericTypes = map[string]bool{ "INT2": true, "INT4": true, "INT8": true, "FLOAT4": true, "FLOAT8": true, "NUMERIC": true, "OID": true, } +var postgresIntegerTypes = map[string]bool{"INT2": true, "INT4": true, "INT8": true} + var postgresSerialTypes = map[string]string{ "smallint": "smallserial", "integer": "serial", @@ -175,7 +177,7 @@ func postgresLiteral(databaseType string, value []byte) string { func postgresSequenceReset(table string, columns []DumpColumn) []string { for _, column := range columns { - if column.Name != postgresIdentityColumn { + if column.Name != postgresIdentityColumn || !postgresIntegerTypes[strings.ToUpper(column.Type)] { continue } quotedTable, quotedColumn := quotePostgresIdentifier(table), quotePostgresIdentifier(column.Name) diff --git a/internal/infrastructure/persistence/repositories/articleRepository.go b/internal/infrastructure/persistence/repositories/articleRepository.go index 66819f2..bb5472a 100644 --- a/internal/infrastructure/persistence/repositories/articleRepository.go +++ b/internal/infrastructure/persistence/repositories/articleRepository.go @@ -46,6 +46,9 @@ func (a *ArticleRepository) scoped(filters domain.Filters) *gorm.DB { if filters.SiteID != nil { query = query.Where("site_id = ?", *filters.SiteID) } + if filters.VisibleTo != nil { + query = query.Where("(is_online = ? OR created_by = ?)", true, *filters.VisibleTo) + } switch filters.Status { case domain.StatusOnline: query = query.Where("is_online = ?", true) @@ -168,6 +171,17 @@ func (a *ArticleRepository) CountByVisibility(onlineOnly bool) (domain.Counts, e return domain.Counts{All: int(online + draft), Online: int(online), Draft: int(draft)}, nil } +func (a *ArticleRepository) CountVisibleTo(userID uint32) (domain.Counts, error) { + var online, draft int64 + if err := a.db.Model(&entity.Article{}).Where("is_online = ?", true).Count(&online).Error; err != nil { + return domain.Counts{}, err + } + if err := a.db.Model(&entity.Article{}).Where("is_online = ? AND created_by = ?", false, userID).Count(&draft).Error; err != nil { + return domain.Counts{}, err + } + return domain.Counts{All: int(online + draft), Online: int(online), Draft: int(draft)}, nil +} + func toDomainArticles(entityArticles []entity.Article) []domain.Article { domainArticles := make([]domain.Article, 0, len(entityArticles)) for _, entityArticle := range entityArticles { @@ -356,11 +370,11 @@ func (a *ArticleRepository) SlugTaken(siteID uint32, slug string, exceptID uint3 return articleSlugInUse(a.db, siteID, slug, exceptID) } -func (a *ArticleRepository) GetAll() []domain.Article { +func (a *ArticleRepository) GetAll() ([]domain.Article, error) { var articles []entity.Article err := a.db.Model(&entity.Article{}).Find(&articles).Error if err != nil { - return []domain.Article{} + return nil, err } var domainArticles = make([]domain.Article, 0) @@ -368,7 +382,7 @@ func (a *ArticleRepository) GetAll() []domain.Article { domainArticles = append(domainArticles, mappers.ArticleToDomain(article)) } - return domainArticles + return domainArticles, nil } func (a *ArticleRepository) UpdateBody(id uint32, body string) (domain.Article, error) { diff --git a/internal/infrastructure/persistence/repositories/entryRepository.go b/internal/infrastructure/persistence/repositories/entryRepository.go index 4cdf4eb..7ffa8f4 100644 --- a/internal/infrastructure/persistence/repositories/entryRepository.go +++ b/internal/infrastructure/persistence/repositories/entryRepository.go @@ -71,7 +71,11 @@ func (e *EntryRepository) ListPage(filters domain.Filters) (domain.PageResult, e filters.Page = max(filters.Page, 1) filters.Size = max(filters.Size, 0) scoped := func() *gorm.DB { - return e.db.Model(&entity.Entry{}).Where("content_type_id = ?", filters.ContentTypeID) + query := e.db.Model(&entity.Entry{}).Where("content_type_id = ?", filters.ContentTypeID) + if filters.CreatedBy != nil { + query = query.Where("created_by = ?", *filters.CreatedBy) + } + return query } var total int64 if err := scoped().Count(&total).Error; err != nil { diff --git a/internal/infrastructure/persistence/repositories/inviteRepository.go b/internal/infrastructure/persistence/repositories/inviteRepository.go index ba824d3..09d1a21 100644 --- a/internal/infrastructure/persistence/repositories/inviteRepository.go +++ b/internal/infrastructure/persistence/repositories/inviteRepository.go @@ -28,16 +28,18 @@ func (i *InviteRepository) Create(invite domainInvite.Invite) (domainInvite.Invi return mappers.InviteToDomain(model), nil } -func (i *InviteRepository) GetAll() []domainInvite.Invite { +func (i *InviteRepository) GetAll() ([]domainInvite.Invite, error) { var invites []entity.Invite - i.db.Model(&entity.Invite{}).Find(&invites) + if err := i.db.Model(&entity.Invite{}).Find(&invites).Error; err != nil { + return nil, err + } var domainInvites []domainInvite.Invite for _, localInvite := range invites { domainInvites = append(domainInvites, mappers.InviteToDomain(localInvite)) } - return domainInvites + return domainInvites, nil } func (i *InviteRepository) Delete(id uint32) error { diff --git a/internal/infrastructure/persistence/repositories/userRepository.go b/internal/infrastructure/persistence/repositories/userRepository.go index 1eff5f9..72c365c 100644 --- a/internal/infrastructure/persistence/repositories/userRepository.go +++ b/internal/infrastructure/persistence/repositories/userRepository.go @@ -52,16 +52,18 @@ func (u *UserRepository) Delete(id uint32) error { return deleteOrNotFound(result, domainUser.ErrUserNotFound) } -func (u *UserRepository) GetAll() []domainUser.User { +func (u *UserRepository) GetAll() ([]domainUser.User, error) { var users []entity.User - u.db.Model(&entity.User{}).Find(&users) + if err := u.db.Model(&entity.User{}).Find(&users).Error; err != nil { + return nil, err + } var domainUsers []domainUser.User for _, localUser := range users { domainUsers = append(domainUsers, mappers.UserToDomain(localUser)) } - return domainUsers + return domainUsers, nil } func (u *UserRepository) FindByIDs(ids []uint32) ([]domainUser.User, error) { diff --git a/internal/infrastructure/useCases/AcceptInvite.go b/internal/infrastructure/useCases/AcceptInvite.go index 85288ae..686472e 100644 --- a/internal/infrastructure/useCases/AcceptInvite.go +++ b/internal/infrastructure/useCases/AcceptInvite.go @@ -72,7 +72,11 @@ func (a *AcceptInviteUseCase) recordRefusal(ctx context.Context, event audit.Eve } func (a *AcceptInviteUseCase) accept(command AcceptInviteCommand, pending invite.Invite, assigned role.Role, now time.Time) (user.User, error) { - if a.emailHasAccount(pending.Email) { + hasAccount, err := a.emailHasAccount(pending.Email) + if err != nil { + return user.User{}, err + } + if hasAccount { if _, err := findUsableInvite(a.inviteRepository, a.hasher, command.Code, now); err != nil { return user.User{}, err } @@ -122,13 +126,17 @@ func (a *AcceptInviteUseCase) inviterOf(pending invite.Invite) (uint32, error) { return assigner.ID, nil } -func (a *AcceptInviteUseCase) emailHasAccount(email string) bool { - for _, existingUser := range a.userRepository.GetAll() { +func (a *AcceptInviteUseCase) emailHasAccount(email string) (bool, error) { + users, err := a.userRepository.GetAll() + if err != nil { + return false, err + } + for _, existingUser := range users { if strings.EqualFold(existingUser.Email, email) { - return true + return true, nil } } - return false + return false, nil } func (a *AcceptInviteUseCase) ensureUsernameFree(username string) error { diff --git a/internal/infrastructure/useCases/CreateInvite.go b/internal/infrastructure/useCases/CreateInvite.go index 8e21e6a..f81af9b 100644 --- a/internal/infrastructure/useCases/CreateInvite.go +++ b/internal/infrastructure/useCases/CreateInvite.go @@ -98,7 +98,11 @@ func (g *CreateInviteUseCase) create(command CreateInviteCommand) (invite.Invite return invite.Invite{}, role.Role{}, err } - if g.emailHasAccount(email) { + hasAccount, err := g.emailHasAccount(email) + if err != nil { + return invite.Invite{}, role.Role{}, err + } + if hasAccount { return invite.Invite{}, role.Role{}, invite.ErrEmailHasAccount } @@ -133,17 +137,25 @@ func (g *CreateInviteUseCase) create(command CreateInviteCommand) (invite.Invite return createdInvite, assigned, nil } -func (g *CreateInviteUseCase) emailHasAccount(email string) bool { - for _, existingUser := range g.userRepository.GetAll() { +func (g *CreateInviteUseCase) emailHasAccount(email string) (bool, error) { + users, err := g.userRepository.GetAll() + if err != nil { + return false, err + } + for _, existingUser := range users { if strings.EqualFold(existingUser.Email, email) { - return true + return true, nil } } - return false + return false, nil } func (g *CreateInviteUseCase) discardOpenInvite(email string, now time.Time) error { - for _, existing := range g.inviteRepository.GetAll() { + invites, err := g.inviteRepository.GetAll() + if err != nil { + return err + } + for _, existing := range invites { if existing.Email != email { continue } diff --git a/internal/infrastructure/useCases/CreateUser.go b/internal/infrastructure/useCases/CreateUser.go index dc30bf7..c2edaa1 100644 --- a/internal/infrastructure/useCases/CreateUser.go +++ b/internal/infrastructure/useCases/CreateUser.go @@ -53,7 +53,11 @@ func (g *CreateUserUseCase) CreateUser(ctx context.Context, createUser CreateUse } func (g *CreateUserUseCase) create(createUser CreateUserCommand) (user.User, error) { - if g.registrationClosed() { + closed, err := g.registrationClosed() + if err != nil { + return user.User{}, err + } + if closed { return user.User{}, user.ErrRegistrationClosed } @@ -108,15 +112,20 @@ func (g *CreateUserUseCase) create(createUser CreateUserCommand) (user.User, err return createdUser, nil } -func (g *CreateUserUseCase) IsRegistrationOpen() bool { - return !g.registrationClosed() +func (g *CreateUserUseCase) IsRegistrationOpen() (bool, error) { + closed, err := g.registrationClosed() + return !closed, err } -func (g *CreateUserUseCase) registrationClosed() bool { - for _, existingUser := range g.userRepository.GetAll() { +func (g *CreateUserUseCase) registrationClosed() (bool, error) { + users, err := g.userRepository.GetAll() + if err != nil { + return false, err + } + for _, existingUser := range users { if existingUser.IsVerified { - return true + return true, nil } } - return false + return false, nil } diff --git a/internal/infrastructure/useCases/EnforceRetention.go b/internal/infrastructure/useCases/EnforceRetention.go index e073246..e373e1e 100644 --- a/internal/infrastructure/useCases/EnforceRetention.go +++ b/internal/infrastructure/useCases/EnforceRetention.go @@ -34,7 +34,11 @@ func (e *EnforceRetentionUseCase) Tick(now time.Time) (retention.Result, error) func (e *EnforceRetentionUseCase) deleteStaleUnverifiedUsers(now time.Time) (int, error) { removed := 0 - for _, candidate := range e.users.GetAll() { + users, err := e.users.GetAll() + if err != nil { + return 0, err + } + for _, candidate := range users { if !candidate.IsStaleUnverifiedAt(now) { continue } @@ -48,7 +52,11 @@ func (e *EnforceRetentionUseCase) deleteStaleUnverifiedUsers(now time.Time) (int func (e *EnforceRetentionUseCase) deleteStaleInvites(now time.Time) (int, error) { removed := 0 - for _, candidate := range e.invites.GetAll() { + invites, err := e.invites.GetAll() + if err != nil { + return 0, err + } + for _, candidate := range invites { if !candidate.IsStaleAt(now) { continue } diff --git a/internal/infrastructure/useCases/GetInvite.go b/internal/infrastructure/useCases/GetInvite.go index 753b4d6..681476e 100644 --- a/internal/infrastructure/useCases/GetInvite.go +++ b/internal/infrastructure/useCases/GetInvite.go @@ -24,7 +24,11 @@ func findUsableInvite(inviteRepository invite.Repository, hasher hash.Hasher, co return invite.Invite{}, invite.ErrInviteInvalid } - for _, candidate := range inviteRepository.GetAll() { + invites, err := inviteRepository.GetAll() + if err != nil { + return invite.Invite{}, err + } + for _, candidate := range invites { if candidate.AcceptedAt != nil { continue } diff --git a/internal/infrastructure/useCases/ListArticles.go b/internal/infrastructure/useCases/ListArticles.go index e7e1066..63bb152 100644 --- a/internal/infrastructure/useCases/ListArticles.go +++ b/internal/infrastructure/useCases/ListArticles.go @@ -42,20 +42,42 @@ type ArticlePage struct { func (u *ListArticlesUseCase) ListArticlesPage(command ListArticlesCommand) (ArticlePage, error) { switch command.Caller.Grants.ScopeFor(role.KeyContentRead, builtintype.KeyArticle) { case role.ScopeAll: - return u.listVisibleByRepository(command, false) + return u.listVisibleByRepository(command, articleVisibility{}) case role.ScopeOwn: - return u.listOwnedAndOnline(command) + return u.listVisibleByRepository(command, articleVisibility{userID: &command.Caller.UserID}) default: - return u.listVisibleByRepository(command, true) + return u.listVisibleByRepository(command, articleVisibility{onlineOnly: true}) } } -func (u *ListArticlesUseCase) listVisibleByRepository(command ListArticlesCommand, onlineOnly bool) (ArticlePage, error) { - counts, err := u.articleRepository.CountByVisibility(onlineOnly) +type articleVisibility struct { + onlineOnly bool + userID *uint32 +} + +func (v articleVisibility) apply(filters article.Filters) article.Filters { + if v.onlineOnly { + online := true + filters.IsOnline = &online + } + filters.VisibleTo = v.userID + return filters +} + +func (u *ListArticlesUseCase) countVisible(visibility articleVisibility) (article.Counts, error) { + if visibility.userID != nil { + return u.articleRepository.CountVisibleTo(*visibility.userID) + } + return u.articleRepository.CountByVisibility(visibility.onlineOnly) +} + +func (u *ListArticlesUseCase) listVisibleByRepository(command ListArticlesCommand, visibility articleVisibility) (ArticlePage, error) { + onlineOnly := visibility.onlineOnly + counts, err := u.countVisible(visibility) if err != nil { return ArticlePage{}, err } - summaries, err := u.summarizeVisible(onlineOnly) + summaries, err := u.summarizeVisible(visibility) if err != nil { return ArticlePage{}, err } @@ -64,7 +86,7 @@ func (u *ListArticlesUseCase) listVisibleByRepository(command ListArticlesComman if onlineOnly && command.Status == article.StatusDraft { return page, nil } - filters := pageFilters(command, onlineOnly) + filters := visibility.apply(pageFilters(command)) if command.SiteKey != nil { siteID, known, err := u.siteIDOf(*command.SiteKey) if err != nil { @@ -98,71 +120,6 @@ func emptyArticlePage(command ListArticlesCommand, counts article.Counts) Articl return ArticlePage{Items: []article.Article{}, Page: command.Page, Size: command.Size, Counts: counts, SEO: map[uint32]article.SEOSummary{}} } -func (u *ListArticlesUseCase) listOwnedAndOnline(command ListArticlesCommand) (ArticlePage, error) { - everything, err := u.articleRepository.FindPage(article.Filters{Unpaged: true}) - if err != nil { - return ArticlePage{}, err - } - visible := visibleTo(command.Caller, everything.Items) - summaries, err := u.summarize(visible) - if err != nil { - return ArticlePage{}, err - } - counts := tally(visible) - counts.Suggestions = countSuggestions(summaries) - page := emptyArticlePage(command, counts) - filters := pageFilters(command, false) - filters.Unpaged = true - if command.SiteKey != nil { - siteID, known, err := u.siteIDOf(*command.SiteKey) - if err != nil { - return ArticlePage{}, err - } - if !known { - return page, nil - } - filters.SiteID = &siteID - } - found, err := u.articleRepository.FindPage(filters) - if err != nil { - return ArticlePage{}, err - } - matching := visibleTo(command.Caller, found.Items) - if command.SEO != "" { - matching = withSEOStatus(matching, summaries, command.SEO) - } - rendered, err := renderArticles(u.bodyRenderer, pageSlice(matching, command.Page, command.Size), command.Format) - if err != nil { - return ArticlePage{}, err - } - page.Items = rendered - page.Total = len(matching) - page.SEO = summaries - return page, nil -} - -func visibleTo(caller ContentCaller, items []article.Article) []article.Article { - visible := make([]article.Article, 0, len(items)) - for _, item := range items { - if caller.seesArticle(item) { - visible = append(visible, item) - } - } - return visible -} - -func tally(visible []article.Article) article.Counts { - counts := article.Counts{All: len(visible)} - for _, item := range visible { - if item.IsOnline { - counts.Online++ - } else { - counts.Draft++ - } - } - return counts -} - func withSEOStatus(items []article.Article, summaries map[uint32]article.SEOSummary, status string) []article.Article { matching := make([]article.Article, 0, len(items)) for _, item := range items { @@ -187,13 +144,8 @@ func pageSlice[T any](items []T, page int, size int) []T { return items[start:min(start+size, len(items))] } -func (u *ListArticlesUseCase) summarizeVisible(onlineOnly bool) (map[uint32]article.SEOSummary, error) { - filters := article.Filters{Unpaged: true} - if onlineOnly { - online := true - filters.IsOnline = &online - } - found, err := u.articleRepository.FindPage(filters) +func (u *ListArticlesUseCase) summarizeVisible(visibility articleVisibility) (map[uint32]article.SEOSummary, error) { + found, err := u.articleRepository.FindPage(visibility.apply(article.Filters{Unpaged: true})) if err != nil { return nil, err } @@ -245,11 +197,6 @@ func (u *ListArticlesUseCase) siteIDOf(key string) (uint32, bool, error) { return found.ID, true, nil } -func pageFilters(command ListArticlesCommand, onlineOnly bool) article.Filters { - filters := article.Filters{Page: command.Page, Size: command.Size, Q: command.Q, Status: command.Status, Sort: command.Sort} - if onlineOnly { - online := true - filters.IsOnline = &online - } - return filters +func pageFilters(command ListArticlesCommand) article.Filters { + return article.Filters{Page: command.Page, Size: command.Size, Q: command.Q, Status: command.Status, Sort: command.Sort} } diff --git a/internal/infrastructure/useCases/ListEntries.go b/internal/infrastructure/useCases/ListEntries.go index 41ad499..7db3050 100644 --- a/internal/infrastructure/useCases/ListEntries.go +++ b/internal/infrastructure/useCases/ListEntries.go @@ -70,7 +70,14 @@ func (u *ListEntriesUseCase) page(owner contenttype.ContentType, command ListEnt if query == "" && readScope == role.ScopeAll { return u.entries.ListPage(entry.Filters{ContentTypeID: owner.ID, Page: command.Page, Size: command.Size, Sort: command.Sort}) } - all, err := u.entries.ListPage(entry.Filters{ContentTypeID: owner.ID, Unpaged: true, Sort: command.Sort}) + if query == "" && readScope == role.ScopeOwn { + return u.entries.ListPage(entry.Filters{ContentTypeID: owner.ID, CreatedBy: &command.Caller.UserID, Page: command.Page, Size: command.Size, Sort: command.Sort}) + } + filters := entry.Filters{ContentTypeID: owner.ID, Unpaged: true, Sort: command.Sort} + if readScope == role.ScopeOwn { + filters.CreatedBy = &command.Caller.UserID + } + all, err := u.entries.ListPage(filters) if err != nil { return entry.PageResult{}, err } diff --git a/internal/infrastructure/useCases/ListUsers.go b/internal/infrastructure/useCases/ListUsers.go index 6a0df14..56dcf87 100644 --- a/internal/infrastructure/useCases/ListUsers.go +++ b/internal/infrastructure/useCases/ListUsers.go @@ -10,6 +10,6 @@ func NewListUsersUseCase(userRepository user.Repository) *ListUsersUseCase { return &ListUsersUseCase{userRepository} } -func (g *ListUsersUseCase) ListUsers() []user.User { +func (g *ListUsersUseCase) ListUsers() ([]user.User, error) { return g.userRepository.GetAll() } diff --git a/internal/infrastructure/useCases/ListUsersAndInvites.go b/internal/infrastructure/useCases/ListUsersAndInvites.go index d46c8bf..1e6f78c 100644 --- a/internal/infrastructure/useCases/ListUsersAndInvites.go +++ b/internal/infrastructure/useCases/ListUsersAndInvites.go @@ -21,8 +21,8 @@ type ListUsersAndInvitesUseCase struct { } type RoleAssigner struct { - ID uint32 `json:"id"` - Name string `json:"name"` + ID uint32 + Name string } func newRoleAssigner(account user.User) *RoleAssigner { @@ -89,7 +89,10 @@ func (g *ListUsersAndInvitesUseCase) ListUsersAndInvites() ([]DirectoryEntry, er now := time.Now() entries := []DirectoryEntry{} - users := g.userRepository.GetAll() + users, err := g.userRepository.GetAll() + if err != nil { + return nil, err + } loaded := make(map[uint32]user.User, len(users)) for _, existingUser := range users { loaded[existingUser.ID] = existingUser @@ -98,7 +101,11 @@ func (g *ListUsersAndInvitesUseCase) ListUsersAndInvites() ([]DirectoryEntry, er entries = append(entries, userEntry(existingUser, lookupRoleRef(refs, existingUser.RoleID), assignerAmong(loaded, existingUser.RoleAssignedByID))) } - for _, existingInvite := range g.inviteRepository.GetAll() { + invites, err := g.inviteRepository.GetAll() + if err != nil { + return nil, err + } + for _, existingInvite := range invites { status := existingInvite.StatusAt(now) if status == invite.StatusAccepted { continue diff --git a/internal/infrastructure/useCases/TransferOwnership.go b/internal/infrastructure/useCases/TransferOwnership.go index cfab3bf..27e8850 100644 --- a/internal/infrastructure/useCases/TransferOwnership.go +++ b/internal/infrastructure/useCases/TransferOwnership.go @@ -21,19 +21,19 @@ type TransferOwnershipCommand struct { } type OwnershipSuccessor struct { - ID uint32 `json:"id"` - Name string `json:"name"` + ID uint32 + Name string } type OwnershipPredecessor struct { - ID uint32 `json:"id"` - Name string `json:"name"` - Role RoleRef `json:"role"` + ID uint32 + Name string + Role RoleRef } type OwnershipTransfer struct { - Owner OwnershipSuccessor `json:"owner"` - FormerOwner OwnershipPredecessor `json:"former_owner"` + Owner OwnershipSuccessor + FormerOwner OwnershipPredecessor } func NewTransferOwnershipUseCase(users user.Repository, roles role.Repository, recorder audit.Recorder) *TransferOwnershipUseCase { diff --git a/internal/infrastructure/useCases/inviteLookup.go b/internal/infrastructure/useCases/inviteLookup.go index 67b9262..18c493b 100644 --- a/internal/infrastructure/useCases/inviteLookup.go +++ b/internal/infrastructure/useCases/inviteLookup.go @@ -3,7 +3,11 @@ package useCases import "RenewCMS/internal/domain/invite" func findManageableInvite(inviteRepository invite.Repository, id uint32) (invite.Invite, error) { - for _, existing := range inviteRepository.GetAll() { + invites, err := inviteRepository.GetAll() + if err != nil { + return invite.Invite{}, err + } + for _, existing := range invites { if existing.ID != id { continue } diff --git a/internal/infrastructure/useCases/roleRef.go b/internal/infrastructure/useCases/roleRef.go index bc0b68f..2509db5 100644 --- a/internal/infrastructure/useCases/roleRef.go +++ b/internal/infrastructure/useCases/roleRef.go @@ -11,10 +11,10 @@ type InviteView struct { } type RoleRef struct { - ID uint32 `json:"id"` - Name string `json:"name"` - BuiltIn bool `json:"built_in"` - Locked bool `json:"locked"` + ID uint32 + Name string + BuiltIn bool + Locked bool } func NewRoleRef(found role.Role) RoleRef { diff --git a/internal/infrastructure/useCases/roleView.go b/internal/infrastructure/useCases/roleView.go index 623eae6..4a658c0 100644 --- a/internal/infrastructure/useCases/roleView.go +++ b/internal/infrastructure/useCases/roleView.go @@ -9,15 +9,15 @@ import ( ) type RoleEditor struct { - ID uint32 `json:"id"` - Name string `json:"name"` + ID uint32 + Name string } type RoleView struct { role.Role - Members int `json:"members"` - UpdatedAt *time.Time `json:"updated_at"` - UpdatedBy *RoleEditor `json:"updated_by"` + Members int + UpdatedAt *time.Time + UpdatedBy *RoleEditor } type roleViewer struct { diff --git a/test/accept_invitation_atomic_test.go b/test/accept_invitation_atomic_test.go index 657c768..2172f23 100644 --- a/test/accept_invitation_atomic_test.go +++ b/test/accept_invitation_atomic_test.go @@ -43,10 +43,10 @@ type atomicInviteRepoDecorator struct { afterGetAll func() } -func (d *atomicInviteRepoDecorator) GetAll() []invite.Invite { - all := d.Repository.GetAll() +func (d *atomicInviteRepoDecorator) GetAll() ([]invite.Invite, error) { + all, err := d.Repository.GetAll() d.afterGetAll() - return all + return all, err } type atomicLookupBarrier struct { diff --git a/test/api_catalogue_drift_test.go b/test/api_catalogue_drift_test.go index 55ed44f..b33dbc6 100644 --- a/test/api_catalogue_drift_test.go +++ b/test/api_catalogue_drift_test.go @@ -50,39 +50,15 @@ func routerRoutes(t *testing.T) map[string]bool { } var routesNotYetInTheCatalogue = []string{ - "DELETE /v1/content-type/{key}", - "DELETE /v1/content-type/{key}/entry/{id}", - "DELETE /v1/image-variants/{name}", - "DELETE /v1/media/{id}/variants/{name}", "DELETE /v1/role/{id}", - "GET /v1/builtin-type", - "GET /v1/builtin-type/{key}", - "GET /v1/content-type", - "GET /v1/content-type/{key}", - "GET /v1/content-type/{key}/entry", - "GET /v1/content-type/{key}/entry/{id}", - "GET /v1/image-variants", - "GET /v1/image-variants/cache", - "GET /v1/media/{id}/variants", "GET /v1/user/lookup", "GET /v1/permission", "GET /v1/role", "GET /v1/role/{id}", - "PATCH /v1/image-variants/{name}", - "POST /v1/content-type", - "POST /v1/content-type/{key}/entry", - "POST /v1/image-variants", - "POST /v1/image-variants/cache/purge", - "POST /v1/image-variants/{name}/purge", - "POST /v1/media/{id}/variants", - "POST /v1/media/{id}/variants/{name}", "POST /v1/role", "POST /v1/role/{id}/reassign", "POST /v1/role/{id}/reset", "POST /v1/site/transfer-ownership", - "PUT /v1/content-type/{key}", - "PUT /v1/content-type/{key}/entry/{id}", - "PUT /v1/image-variants/{name}", "PUT /v1/role/{id}", "PUT /v1/user/{id}/role", } diff --git a/test/article_test.go b/test/article_test.go index 3072147..4e6264b 100644 --- a/test/article_test.go +++ b/test/article_test.go @@ -92,6 +92,9 @@ func (m *mockArticleRepo) FindPage(filters article.Filters) (article.PageResult, if filters.SiteID != nil && a.SiteID != *filters.SiteID { continue } + if filters.VisibleTo != nil && !a.IsOnline && a.CreatedBy != *filters.VisibleTo { + continue + } if filters.Status == article.StatusOnline && !a.IsOnline { continue } @@ -132,6 +135,22 @@ func (m *mockArticleRepo) CountByVisibility(onlineOnly bool) (article.Counts, er return counts, nil } +func (m *mockArticleRepo) CountVisibleTo(userID uint32) (article.Counts, error) { + if err := m.fail.trigger("CountVisibleTo"); err != nil { + return article.Counts{}, err + } + counts := article.Counts{} + for _, a := range m.articles { + if a.IsOnline { + counts.Online++ + } else if a.CreatedBy == userID { + counts.Draft++ + } + } + counts.All = counts.Online + counts.Draft + return counts, nil +} + func articleSortsBefore(left, right article.Article, order article.Sort) bool { if order.Field == "" { order = article.DefaultSort() @@ -195,12 +214,12 @@ func (m *mockArticleRepo) GetByName(name string) (article.Article, error) { return article.Article{}, article.ErrArticleNotFound } -func (m *mockArticleRepo) GetAll() []article.Article { +func (m *mockArticleRepo) GetAll() ([]article.Article, error) { result := make([]article.Article, 0) for _, id := range m.sortedIds() { result = append(result, m.articles[id]) } - return result + return result, nil } func (m *mockArticleRepo) slugUsedOn(siteID uint32, slug string, exceptID uint32) bool { diff --git a/test/custom_content_types_support_test.go b/test/custom_content_types_support_test.go index 599eb0a..da234ec 100644 --- a/test/custom_content_types_support_test.go +++ b/test/custom_content_types_support_test.go @@ -146,7 +146,7 @@ func (m *mockEntryRepo) ListPage(filters entry.Filters) (entry.PageResult, error } matching := []entry.Entry{} for _, stored := range m.entries { - if stored.ContentTypeID == filters.ContentTypeID { + if stored.ContentTypeID == filters.ContentTypeID && (filters.CreatedBy == nil || stored.CreatedBy == *filters.CreatedBy) { matching = append(matching, stored) } } diff --git a/test/get_all_error_propagation_test.go b/test/get_all_error_propagation_test.go new file mode 100644 index 0000000..d76699a --- /dev/null +++ b/test/get_all_error_propagation_test.go @@ -0,0 +1,74 @@ +package test + +import ( + "RenewCMS/internal/infrastructure/useCases" + "errors" + "net/http" + "testing" + "time" +) + +func TestGetAllFailuresReachTheCaller(t *testing.T) { + boom := errors.New("storage down") + + t.Run("registration status answers 500 instead of reporting open", func(t *testing.T) { + userRepo := newMockUserRepo() + userRepo.fail.failNextWith("GetAll", boom) + router, _ := newRegistrationStatusRouter(userRepo, newMockMailRepo()) + + rec := registrationStatusRequest(router, nil) + + if rec.Code != http.StatusInternalServerError { + t.Errorf("status = %d, want 500", rec.Code) + } + }) + + t.Run("IsRegistrationOpen returns the error", func(t *testing.T) { + userRepo := newMockUserRepo() + userRepo.fail.failNextWith("GetAll", boom) + uc := useCases.NewCreateUserUseCase(userRepo, newSeededMockRoleRepo(), &stubHasher{}, newMockMailRepo(), "https://cms.example.com", discardRecorder) + + if _, err := uc.IsRegistrationOpen(); !errors.Is(err, boom) { + t.Errorf("err = %v, want %v", err, boom) + } + }) + + t.Run("GetInvite returns the error instead of an invalid invite", func(t *testing.T) { + invites := newInviteFakeRepo() + invites.fail.failNextWith("GetAll", boom) + uc := useCases.NewGetInviteUseCase(invites, &stubHasher{}) + + if _, err := uc.GetInvite("code"); !errors.Is(err, boom) { + t.Errorf("err = %v, want %v", err, boom) + } + }) + + t.Run("ListUsers returns the error", func(t *testing.T) { + userRepo := newMockUserRepo() + userRepo.fail.failNextWith("GetAll", boom) + + if _, err := useCases.NewListUsersUseCase(userRepo).ListUsers(); !errors.Is(err, boom) { + t.Errorf("err = %v, want %v", err, boom) + } + }) + + t.Run("retention stops on a user listing failure", func(t *testing.T) { + userRepo := newMockUserRepo() + userRepo.fail.failNextWith("GetAll", boom) + uc := useCases.NewEnforceRetentionUseCase(userRepo, newInviteFakeRepo(), &memorySessionRepo{}) + + if _, err := uc.Tick(time.Now()); !errors.Is(err, boom) { + t.Errorf("err = %v, want %v", err, boom) + } + }) + + t.Run("retention stops on an invite listing failure", func(t *testing.T) { + invites := newInviteFakeRepo() + invites.fail.failNextWith("GetAll", boom) + uc := useCases.NewEnforceRetentionUseCase(newMockUserRepo(), invites, &memorySessionRepo{}) + + if _, err := uc.Tick(time.Now()); !errors.Is(err, boom) { + t.Errorf("err = %v, want %v", err, boom) + } + }) +} diff --git a/test/invitations_test.go b/test/invitations_test.go index af85166..4599b1a 100644 --- a/test/invitations_test.go +++ b/test/invitations_test.go @@ -52,7 +52,10 @@ func (m *inviteFakeRepo) Create(i invite.Invite) (invite.Invite, error) { return m.seed(i), nil } -func (m *inviteFakeRepo) GetAll() []invite.Invite { +func (m *inviteFakeRepo) GetAll() ([]invite.Invite, error) { + if err := m.fail.trigger("GetAll"); err != nil { + return nil, err + } ids := make([]uint32, 0, len(m.invites)) for id := range m.invites { ids = append(ids, id) @@ -62,7 +65,7 @@ func (m *inviteFakeRepo) GetAll() []invite.Invite { for _, id := range ids { result = append(result, m.invites[id]) } - return result + return result, nil } func (m *inviteFakeRepo) Delete(id uint32) error { diff --git a/test/mysql_server_test.go b/test/mysql_server_test.go index 18857dd..217bbf1 100644 --- a/test/mysql_server_test.go +++ b/test/mysql_server_test.go @@ -59,8 +59,9 @@ func TestMySQLMigration(t *testing.T) { } } binaryTables := columnCount(t, db, "SELECT COUNT(*) FROM information_schema.tables WHERE table_schema = DATABASE() AND table_collation = 'utf8mb4_bin'") - if binaryTables != 6 { - t.Errorf("every table must use the utf8mb4_bin collation, got %d of 6", binaryTables) + allTables := columnCount(t, db, "SELECT COUNT(*) FROM information_schema.tables WHERE table_schema = DATABASE() AND table_type = 'BASE TABLE'") + if binaryTables != allTables { + t.Errorf("every table must use the utf8mb4_bin collation, got %d of %d", binaryTables, allTables) } }) @@ -247,7 +248,7 @@ func TestMySQLBackup(t *testing.T) { if line := firstDifferingLine(withoutRedundantCharset(dump), withoutRedundantCharset(string(restoredContent))); line != "" { t.Errorf("replaying the dump must restore the same schema and rows, first difference: %s", line) } - restoredUsers := newContractRepositories(restored.DB).users.GetAll() + restoredUsers := allOf(newContractRepositories(restored.DB).users.GetAll()) if len(restoredUsers) != 2 || restoredUsers[0].Username != "o'brien \"quoted\" back\\slash\nline é😀" || restoredUsers[1].PasswordResetCode != "reset-code" { t.Errorf("unexpected restored users %+v", restoredUsers) } diff --git a/test/own_scope_visibility_persistence_test.go b/test/own_scope_visibility_persistence_test.go new file mode 100644 index 0000000..392b934 --- /dev/null +++ b/test/own_scope_visibility_persistence_test.go @@ -0,0 +1,111 @@ +package test + +import ( + "RenewCMS/internal/domain/article" + "RenewCMS/internal/domain/contenttype" + "RenewCMS/internal/domain/entry" + "RenewCMS/internal/infrastructure/persistence/database" + persistence "RenewCMS/internal/infrastructure/persistence/repositories" + "testing" +) + +func TestArticleRepositoryVisibilityFilterOnSQLite(t *testing.T) { + db := openContractSQLite(t) + repo := persistence.NewArticleRepository(db) + defaultSite, err := persistence.NewSiteRepository(db).GetDefault() + if err != nil { + t.Fatal(err) + } + create := func(owner uint32, online bool) { + t.Helper() + record := articleOn(defaultSite.ID) + record.CreatedBy = owner + record.IsOnline = online + if _, err := repo.Create(record); err != nil { + t.Fatal(err) + } + } + create(1, true) + create(1, false) + create(2, true) + create(2, false) + create(2, false) + one := uint32(1) + + t.Run("FindPage keeps online articles and the viewer's drafts", func(t *testing.T) { + found, err := repo.FindPage(article.Filters{VisibleTo: &one, Unpaged: true}) + if err != nil { + t.Fatal(err) + } + if found.Total != 3 || len(found.Items) != 3 { + t.Fatalf("total = %d, items = %d, want 3", found.Total, len(found.Items)) + } + for _, item := range found.Items { + if !item.IsOnline && item.CreatedBy != one { + t.Errorf("leaked draft %+v", item) + } + } + }) + + t.Run("FindPage pages the filtered set", func(t *testing.T) { + found, err := repo.FindPage(article.Filters{VisibleTo: &one, Page: 2, Size: 2}) + if err != nil { + t.Fatal(err) + } + if found.Total != 3 || len(found.Items) != 1 { + t.Fatalf("total = %d, items = %d, want 3 and 1", found.Total, len(found.Items)) + } + }) + + t.Run("FindPage combines the filter with the draft status", func(t *testing.T) { + found, err := repo.FindPage(article.Filters{VisibleTo: &one, Status: article.StatusDraft, Unpaged: true}) + if err != nil { + t.Fatal(err) + } + if found.Total != 1 || found.Items[0].CreatedBy != one { + t.Fatalf("found = %+v", found) + } + }) + + t.Run("CountVisibleTo counts online articles and the viewer's drafts", func(t *testing.T) { + counts, err := repo.CountVisibleTo(one) + if err != nil { + t.Fatal(err) + } + if counts != (article.Counts{All: 3, Online: 2, Draft: 1}) { + t.Fatalf("counts = %+v", counts) + } + }) +} + +func TestEntryRepositoryCreatedByFilterOnSQLite(t *testing.T) { + db := openUnmigratedSQLite(t) + if err := database.Migrate(db, migrationPublicURL); err != nil { + t.Fatal(err) + } + types := persistence.NewContentTypeRepository(db) + entries := persistence.NewEntryRepository(db) + created, err := types.Create(contenttype.ContentType{Key: "notes", Name: "Notes", Fields: []contenttype.Field{{Key: "flag", Label: "Flag", Type: contenttype.TypeBoolean}}}) + if err != nil { + t.Fatal(err) + } + for _, owner := range []uint32{1, 2, 1, 2, 1} { + if _, err := entries.Create(entry.Entry{ContentTypeID: created.ID, Data: map[string]any{}, CreatedBy: owner, UpdatedBy: owner}); err != nil { + t.Fatal(err) + } + } + one := uint32(1) + + found, err := entries.ListPage(entry.Filters{ContentTypeID: created.ID, CreatedBy: &one, Page: 1, Size: 2, Sort: entry.Sort{Field: entry.SortByID, Direction: entry.SortAsc}}) + if err != nil { + t.Fatal(err) + } + if found.Total != 3 || len(found.Items) != 2 { + t.Fatalf("total = %d, items = %d, want 3 and 2", found.Total, len(found.Items)) + } + for _, item := range found.Items { + if item.CreatedBy != one { + t.Errorf("leaked entry %+v", item) + } + } +} diff --git a/test/personal_data_retention_test.go b/test/personal_data_retention_test.go index f709421..67df702 100644 --- a/test/personal_data_retention_test.go +++ b/test/personal_data_retention_test.go @@ -108,7 +108,7 @@ func TestEnforceRetentionUseCase(t *testing.T) { if _, removed := w.users.users[2]; removed { t.Error("the abandoned account must be removed") } - remaining := w.invites.GetAll() + remaining := allOf(w.invites.GetAll()) if len(remaining) != 1 || remaining[0].Email != "pending@example.com" { t.Errorf("expected only the pending invite kept, got %+v", remaining) } diff --git a/test/postgres_server_test.go b/test/postgres_server_test.go index b194212..ecd05d3 100644 --- a/test/postgres_server_test.go +++ b/test/postgres_server_test.go @@ -185,7 +185,7 @@ func TestPostgresMigration(t *testing.T) { }) t.Run("a legacy users.role column is backfilled into role_id and the invites get Admin", func(t *testing.T) { - _, _, db := migratedPostgres(t) + server, config, db := migratedPostgres(t) addLegacyRoleColumn(t, db) for _, name := range []string{"first", "second", "third"} { insert := db.Exec("INSERT INTO users (username, password, email, role, is_verified, token_version) VALUES (?, 'x', ?, '', true, 0)", name, name+"@x.example") @@ -193,6 +193,7 @@ func TestPostgresMigration(t *testing.T) { t.Fatalf("insert %s: %v", name, insert.Error) } } + db = server.connect(t, config).DB if err := database.Migrate(db, migrationPublicURL); err != nil { t.Fatalf("migrate: %v", err) @@ -241,7 +242,7 @@ func TestPostgresTimesInLists(t *testing.T) { mustCreateUser(t, repos, user) } - users := repos.users.GetAll() + users := allOf(repos.users.GetAll()) if len(users) != 2 { t.Fatalf("expected 2 users, got %d", len(users)) @@ -402,7 +403,7 @@ func TestPostgresBackup(t *testing.T) { t.Errorf("replaying the dump must restore the same schema and rows, first difference: %s", line) } restoredRepos := newContractRepositories(restored.DB) - restoredUsers := restoredRepos.users.GetAll() + restoredUsers := allOf(restoredRepos.users.GetAll()) if len(restoredUsers) != 2 || restoredUsers[0].Username != "o'brien \"quoted\" back\\slash\nline é😀" || restoredUsers[1].PasswordResetCode != "reset-code" { t.Errorf("unexpected restored users %+v", restoredUsers) } diff --git a/test/registration_status_endpoint_http_test.go b/test/registration_status_endpoint_http_test.go index 6c27053..a2d3331 100644 --- a/test/registration_status_endpoint_http_test.go +++ b/test/registration_status_endpoint_http_test.go @@ -164,7 +164,7 @@ func TestRegistrationStatusEndpointHttp(t *testing.T) { mailRepo := newMockMailRepo() uc := useCases.NewCreateUserUseCase(userRepo, newSeededMockRoleRepo(), &stubHasher{}, mailRepo, "https://cms.example.com", discardRecorder) - if uc.IsRegistrationOpen() { + if open, openErr := uc.IsRegistrationOpen(); openErr != nil || open { t.Fatalf("expected IsRegistrationOpen to return false") } diff --git a/test/repository_contract_test.go b/test/repository_contract_test.go index 3f09602..f9b4406 100644 --- a/test/repository_contract_test.go +++ b/test/repository_contract_test.go @@ -207,7 +207,7 @@ func runRepositoryContract(t *testing.T, backend contractBackend) { if err == nil { t.Error("expected an error for the duplicate email") } - if all := repos.users.GetAll(); len(all) != 1 || all[0].Username != "first" { + if all := allOf(repos.users.GetAll()); len(all) != 1 || all[0].Username != "first" { t.Errorf("nothing may be stored for the failed create, got %+v", all) } }}, @@ -231,7 +231,7 @@ func runRepositoryContract(t *testing.T, backend contractBackend) { if !errors.Is(secondErr, domainInvite.ErrInviteNotFound) { t.Errorf("expected ErrInviteNotFound, got %v", secondErr) } - if len(repos.users.GetAll()) != 1 { + if len(allOf(repos.users.GetAll())) != 1 { t.Errorf("only the first accept may create a user") } }}, @@ -244,7 +244,7 @@ func runRepositoryContract(t *testing.T, backend contractBackend) { if err == nil { t.Fatal("expected the taken username to be refused") } - invites := repos.invites.GetAll() + invites := allOf(repos.invites.GetAll()) if len(invites) != 1 || invites[0].AcceptedAt != nil { t.Errorf("the invite must stay pending, got %+v", invites) } @@ -366,3 +366,10 @@ func TestSQLiteRepositoryContract(t *testing.T) { func TestMySQLRepositoryContract(t *testing.T) { runRepositoryContract(t, contractBackend{name: "mysql", open: openContractMySQL}) } + +func allOf[T any](all []T, err error) []T { + if err != nil { + panic(err) + } + return all +} diff --git a/test/site_cleanup_test.go b/test/site_cleanup_test.go index 721e281..f95c297 100644 --- a/test/site_cleanup_test.go +++ b/test/site_cleanup_test.go @@ -165,20 +165,20 @@ func TestArticleRepositoryWritesOnSQLite(t *testing.T) { } } - if all := repo.GetAll(); len(all) != 2 { + if all := allOf(repo.GetAll()); len(all) != 2 { t.Errorf("articles = %+v", all) } }) - t.Run("GetAll returns an empty list when the query fails", func(t *testing.T) { + t.Run("GetAll returns the error when the query fails", func(t *testing.T) { repo, db, _ := setup(t) fault := injectDatabaseFault(db, "fault:getall", faultOnQuery) fault.armed.Store(true) - all := repo.GetAll() + all, err := repo.GetAll() - if all == nil || len(all) != 0 { - t.Errorf("articles = %#v", all) + if err == nil || len(all) != 0 { + t.Errorf("articles = %#v, err = %v", all, err) } }) @@ -436,7 +436,7 @@ func TestSiteDeletionIsCheckedInsideTheTransaction(t *testing.T) { _, updateErr := articles.Update(created.ID, article.Update{SiteID: &shop.ID}) unchanged, _ := articles.Get(created.ID) - if !errors.Is(createErr, site.ErrSiteNotFound) || !errors.Is(updateErr, site.ErrSiteNotFound) || unchanged.SiteID != blog.ID || len(articles.GetAll()) != 1 { + if !errors.Is(createErr, site.ErrSiteNotFound) || !errors.Is(updateErr, site.ErrSiteNotFound) || unchanged.SiteID != blog.ID || len(allOf(articles.GetAll())) != 1 { t.Errorf("create %v, update %v, site %d", createErr, updateErr, unchanged.SiteID) } }) diff --git a/test/unverified_users_and_reregistration_test.go b/test/unverified_users_and_reregistration_test.go index a91fb14..4bf6302 100644 --- a/test/unverified_users_and_reregistration_test.go +++ b/test/unverified_users_and_reregistration_test.go @@ -113,7 +113,7 @@ func TestUnverifiedUsersAndReregistrationRepository(t *testing.T) { t.Fatal("expected Create to fail because the username is still in use") } - users := repo.GetAll() + users := allOf(repo.GetAll()) count := 0 for _, u := range users { if u.Username == "alice" { diff --git a/test/use_case_baseline_test.go b/test/use_case_baseline_test.go index 43c3856..bc20ab9 100644 --- a/test/use_case_baseline_test.go +++ b/test/use_case_baseline_test.go @@ -549,7 +549,7 @@ func TestListUsersReturnsEveryUser(t *testing.T) { repo.users[2] = user.User{ID: 2} uc := useCases.NewListUsersUseCase(repo) - result := uc.ListUsers() + result, _ := uc.ListUsers() if len(result) != 2 { t.Fatalf("expected 2 users, got %d", len(result)) diff --git a/test/user_test.go b/test/user_test.go index eff906f..90fc814 100644 --- a/test/user_test.go +++ b/test/user_test.go @@ -115,13 +115,16 @@ func (m *mockUserRepo) GetByEmail(email string) (user.User, error) { return user.User{}, user.ErrUserNotFound } -func (m *mockUserRepo) GetAll() []user.User { +func (m *mockUserRepo) GetAll() ([]user.User, error) { m.getAllCalls++ + if err := m.fail.trigger("GetAll"); err != nil { + return nil, err + } result := make([]user.User, 0) for _, id := range m.sortedIds() { result = append(result, m.users[id]) } - return result + return result, nil } func (m *mockUserRepo) FindByIDs(ids []uint32) ([]user.User, error) { diff --git a/test/users_api_hardening_test.go b/test/users_api_hardening_test.go index 39cbc39..f1d9634 100644 --- a/test/users_api_hardening_test.go +++ b/test/users_api_hardening_test.go @@ -260,7 +260,7 @@ type usersAPIGetAllHookRepo struct { hook func() } -func (r *usersAPIGetAllHookRepo) GetAll() []user.User { +func (r *usersAPIGetAllHookRepo) GetAll() ([]user.User, error) { if r.hook != nil { hook := r.hook r.hook = nil diff --git a/test/users_api_persistence_test.go b/test/users_api_persistence_test.go index 6ef2885..3974898 100644 --- a/test/users_api_persistence_test.go +++ b/test/users_api_persistence_test.go @@ -167,7 +167,7 @@ func TestUsersAPIInviteRepository(t *testing.T) { if err != nil || renewed.RoleID != writer { t.Fatalf("unexpected %+v, %v", renewed, err) } - if all := repo.GetAll(); len(all) != 1 || all[0].RoleID != writer { + if all := allOf(repo.GetAll()); len(all) != 1 || all[0].RoleID != writer { t.Errorf("unexpected %+v", all) } }) diff --git a/web/src/components/ApiSearchPalette.vue b/web/src/components/ApiSearchPalette.vue index 3632c92..3741513 100644 --- a/web/src/components/ApiSearchPalette.vue +++ b/web/src/components/ApiSearchPalette.vue @@ -1,10 +1,10 @@ diff --git a/web/src/components/ContentToolbar.vue b/web/src/components/ContentToolbar.vue index 830172b..90dc75b 100644 --- a/web/src/components/ContentToolbar.vue +++ b/web/src/components/ContentToolbar.vue @@ -1,13 +1,11 @@